Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address Feedback #43

Open
8 tasks done
shashankhs11 opened this issue Jan 30, 2025 · 2 comments
Open
8 tasks done

Address Feedback #43

shashankhs11 opened this issue Jan 30, 2025 · 2 comments
Milestone

Comments

@shashankhs11
Copy link
Collaborator

shashankhs11 commented Jan 30, 2025

  • Function specification ordering and coding style varies between functions. - Already fixed (Marek)
  • Your functions are all sorting algorithms, please make function consistent with inputs and outputs. Already fixed (Marek)
  • There are unnecessary files present in the repo. pysorting.py is unnecessary. Already fixed (Marek)
  • did not list all team members in the author lists in pyproject.toml, LICENSE. Already fixed (Shashank)
  • One or more documentations are missing one or more required elements (title, argument, returned, example)(- 1 * total number of cases). Already fixed.
  • Functions in utils need context or examples. (Nonso)
  • Shellsort: weird pass post return. Already fixed (Sid)
  • Sorting functions have have a coherent test function design since all functions should have the same outputs when given the same inputs regardless of input. Current design is messy. (Sid)
@shashankhs11
Copy link
Collaborator Author

shashankhs11 commented Jan 31, 2025

Also need to address the feedback from the review comments lister here -
UBC-MDS/software-review-2025#29

Expanding the feedback for better tracking

  • The readme file did not include documentation for tests. It would be better if those are included for better clarity. (Shashank)
  • There is only one badge in the readme file and the test coverage is not included. (Nonso)
  • The actions in workflow are not passed. (Old issue, not valid)
  • There is no citation in the Readme file. (Not exactly sure what is required for this - Marek)
  • It would be better if the contributors emails are included. (Not addressing this)
  • Something I wanted to mention is that your README.md installation instructions seems to suggest that we can pip install directly directly using pip install pysorting, but your package isn't yet on Pypi. I think what you meant to instruct was to cd dist into your dist folder after cloning and running pip install pysorting-0.1.0.tar.gz That being said, for milestone 4 you will be publishing to pypi so your README instructions will end up being okay! No fix needed I think in the meantime. (Nonso/Marek)
  • On the README note, under Usage I think you may have accidentally left in an old note, specifically "...Please note that the functions are currently not implemented..." (Marek)
  • Also within the README, I couldn't tick all the badges, but I did see your documentation badge is there! I'm not fully sure if these other badges are required as they weren't explicitly mentioned in past milestones, something to discuss with your team though! (Shashank)
  • Final thing about the README, although you guys do have the badge to your docs, the reviewer checklist would like to see a descriptive link to the vignette/example.ipynb file on your readthedocs link. (Nonso)
  • Last thing, I think you may have accidentally left in an old deprecated file called test_pysorting. When I try to run pytest tests/ I get the following error: (Marek)

@shashankhs11
Copy link
Collaborator Author

shashankhs11 commented Feb 3, 2025

  • COV badge and ci-cd badge (Nonso)
  • Choose a suitable license to your package and justify your choice (Marek)

@shashankhs11 shashankhs11 added this to the Milestone 4 milestone Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

1 participant