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

Group 15: num_theory #18

Open
11 of 29 tasks
vrudhgarg opened this issue Jan 28, 2025 · 9 comments
Open
11 of 29 tasks

Group 15: num_theory #18

vrudhgarg opened this issue Jan 28, 2025 · 9 comments

Comments

@vrudhgarg
Copy link

vrudhgarg commented Jan 28, 2025

Submitting Author: Dhruv Garg (@vrudhgarg)
All current maintainers: (@ThamerD, @dominic-lam @Calista321)
Package Name: num_theory
One-Line Description of Package: A high-performance Python package for number theory operations, optimized for Project Euler and computational mathematics problems.
Repository Link: https://github.com/UBC-MDS/num_theory
Version submitted: 1.1.0
Editor: TBD
Reviewer 1: Jason Lee
Reviewer 2: Yibin Long
Reviewer 3: Adrian Leung
Archive: TBD
JOSS DOI: TBD
Version accepted: TBD
Date accepted (month/day/year): TBD


Code of Conduct & Commitment to Maintain Package

Description

  • This package provides essential number theory functions designed for educational and analytical purposes. From prime factorization to generating arithmetic progressions, the num_theory package is a versatile tool for students, researchers, and enthusiasts alike. It can also serve as a utility for developing solutions to Project Euler problems.

Scope

  • Please indicate which category or categories.
    Check out our package scope page to learn more about our
    scope. (If you are unsure of which category you fit, we suggest you make a pre-submission inquiry):

    • Data retrieval
    • Data extraction
    • Data processing/munging
    • Data deposition
    • Data validation and testing
    • Data visualization1
    • Workflow automation
    • Citation management and bibliometrics
    • Scientific software wrappers
    • Database interoperability

Domain Specific & Community Partnerships

- [ ] Geospatial
- [x] Education
- [ ] Pangeo

Community Partnerships

If your package is associated with an
existing community please check below:

  • For all submissions, explain how the and why the package falls under the categories you indicated above. In your explanation, please address the following points (briefly, 1-2 sentences for each):

    • Who is the target audience and what are scientific applications of this package?
      The num_theory package is a versatile tool for students, researchers, and enthusiasts alike

    • Are there other Python packages that accomplish the same thing? If so, how does yours differ?
      Libraries like SymPy (symbolic math), NumPy (numerical computing), and primesieve (prime generation) overlap in functionality with our package, but num_theory is specifically optimized for Project Euler problems, offering a simple, focused set of number theory tools (e.g., fast prime checks, modular arithmetic) with a gentle learning curve.

    • If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted:

Technical checks

For details about the pyOpenSci packaging requirements, see our packaging guide. Confirm each of the following by checking the box. This package:

  • does not violate the Terms of Service of any service it interacts with.
  • uses an OSI approved license.
  • contains a README with instructions for installing the development version.
  • includes documentation with examples for all functions.
  • contains a tutorial with examples of its essential functions and uses.
  • has a test suite.
  • has continuous integration setup, such as GitHub Actions CircleCI, and/or others.

Publication Options

JOSS Checks
  • The package has an obvious research application according to JOSS's definition in their submission requirements. Be aware that completing the pyOpenSci review process does not guarantee acceptance to JOSS. Be sure to read their submission requirements (linked above) if you are interested in submitting to JOSS.
  • The package is not a "minor utility" as defined by JOSS's submission requirements: "Minor ‘utility’ packages, including ‘thin’ API clients, are not acceptable." pyOpenSci welcomes these packages under "Data Retrieval", but JOSS has slightly different criteria.
  • The package contains a paper.md matching JOSS's requirements with a high-level description in the package root or in inst/.
  • The package is deposited in a long-term repository with the DOI:

Note: JOSS accepts our review as theirs. You will NOT need to go through another full review. JOSS will only review your paper.md file. Be sure to link to this pyOpenSci issue when a JOSS issue is opened for your package. Also be sure to tell the JOSS editor that this is a pyOpenSci reviewed package once you reach this step.

Are you OK with Reviewers Submitting Issues and/or pull requests to your Repo Directly?

This option will allow reviewers to open smaller issues that can then be linked to PR's rather than submitting a more dense text based review. It will also allow you to demonstrate addressing the issue via PR links.

  • Yes I am OK with reviewers submitting requested changes as issues to my repo. Reviewers will then link to the issues in their submitted review.

Confirm each of the following by checking the box.

  • I have read the author guide.
  • I expect to maintain this package for at least 2 years and can help find a replacement for the maintainer (team) if needed.

P.S. Have feedback/comments about our review process? Leave a comment here

Editor and Review Templates

The editor template can be found here.

The review template can be found here.

Footnotes

  1. Please fill out a pre-submission inquiry before submitting a data visualization package.

@adrianl726
Copy link

adrianl726 commented Jan 29, 2025

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README.
  • Installation instructions: for the development version of the package and any non-standard dependencies in README.
  • Vignette(s) demonstrating major functionality that runs successfully locally.
  • Function Documentation: for all user-facing functions.
  • Examples for all user-facing functions.
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING.
  • Metadata including author(s), author e-mail(s), a url, and any other relevant metadata e.g., in a pyproject.toml file or elsewhere.

Readme file requirements
The package meets the readme requirements below:

  • Package has a README.md file in the root directory.

The README should include, from top to bottom:

  • The package name
  • Badges for:
    • Continuous integration and test coverage,
    • Docs building (if you have a documentation website),
    • A repostatus.org badge,
    • Python versions supported,
    • Current package version (on PyPI / Conda).

NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)

  • Short description of package goals.
  • Package installation instructions
  • Any additional setup required to use the package (authentication tokens, etc.)
  • Descriptive links to all vignettes. If the package is small, there may only be a need for one vignette which could be placed in the README.md file.
    • Brief demonstration of package usage (as it makes sense - links to vignettes could also suffice here if package description is clear)
  • Link to your documentation website.
  • If applicable, how the package compares to other similar packages and/or how it relates to other packages in the scientific ecosystem.
  • Citation information

Usability

Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole.
Package structure should follow general community best-practices. In general please consider whether:

  • Package documentation is clear and easy to find and use.
  • The need for the package is clear
  • All functions have documentation and associated examples for use
  • The package is easy to install

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests:
    • All tests pass on the reviewer's local machine for the package version submitted by the author. Ideally this should be a tagged version making it easy for reviewers to install.
    • Tests cover essential functions of the package and a reasonable range of inputs and conditions.
  • Continuous Integration: Has continuous integration setup (We suggest using Github actions but any CI platform is acceptable for review)
  • Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.
    A few notable highlights to look at:
    • Package supports modern versions of Python and not End of life versions.
    • Code format is standard throughout package and follows PEP 8 guidelines (CI tests for linting pass)

For packages also submitting to JOSS

Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.

The package contains a paper.md matching JOSS's requirements with:

  • A short summary describing the high-level functionality of the software
  • Authors: A list of authors with their affiliations
  • A statement of need clearly stating problems the software is designed to solve and its target audience.
  • References: With DOIs for all those that have one (e.g. papers, datasets, software).

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 1 hour


Review Comments

This is a well-documented package with interesting and unique ideas. I am impressed with what this group has achieved. These are a few improvements that I think will enhance your package even more:

  • The README.md title should be your package name num_project instead of 'Number Theory'.
  • It would be nice to have a brief description on 'Project Euler', especially for readers that do not have experience with Project Euler.
  • Include a few examples of computational tasks that num_project can achieve in the introduction in README.md.
  • Also include the target audience of num_project in the introduction.
  • Add badges for ci-cd, python version, and current package version to README.md.

Keep up with the good work!

@jasonmlee
Copy link

jasonmlee commented Jan 29, 2025

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README.
  • Installation instructions: for the development version of the package and any non-standard dependencies in README.
  • Vignette(s) demonstrating major functionality that runs successfully locally.
  • Function Documentation: for all user-facing functions.
  • Examples for all user-facing functions.
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING.
  • Metadata including author(s), author e-mail(s), a url, and any other relevant metadata e.g., in a pyproject.toml file or elsewhere.

Readme file requirements
The package meets the readme requirements below:

  • Package has a README.md file in the root directory.

The README should include, from top to bottom:

  • The package name
  • Badges for:
    • Continuous integration and test coverage,
    • Docs building (if you have a documentation website),
    • A repostatus.org badge,
    • Python versions supported,
    • Current package version (on PyPI / Conda).

NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)

  • Short description of package goals.
  • Package installation instructions
  • Any additional setup required to use the package (authentication tokens, etc.)
  • Descriptive links to all vignettes. If the package is small, there may only be a need for one vignette which could be placed in the README.md file.
    • Brief demonstration of package usage (as it makes sense - links to vignettes could also suffice here if package description is clear)
  • Link to your documentation website.
  • If applicable, how the package compares to other similar packages and/or how it relates to other packages in the scientific ecosystem.
  • Citation information

Usability

Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole.
Package structure should follow general community best-practices. In general please consider whether:

  • Package documentation is clear and easy to find and use.
  • The need for the package is clear
  • All functions have documentation and associated examples for use
  • The package is easy to install

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests:
  • All tests pass on the reviewer's local machine for the package version submitted by the author. Ideally this should be a tagged version making it easy for reviewers to install.
  • Tests cover essential functions of the package and a reasonable range of inputs and conditions.
  • Continuous Integration: Has continuous integration setup (We suggest using Github actions but any CI platform is acceptable for review)
  • Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.
    A few notable highlights to look at:
    • Package supports modern versions of Python and not End of life versions.
    • Code format is standard throughout package and follows PEP 8 guidelines (CI tests for linting pass)

For packages also submitting to JOSS

Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.

The package contains a paper.md matching JOSS's requirements with:

  • A short summary describing the high-level functionality of the software
  • Authors: A list of authors with their affiliations
  • A statement of need clearly stating problems the software is designed to solve and its target audience.
  • References: With DOIs for all those that have one (e.g. papers, datasets, software).

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing:
1.5 hours

Review Comments

Really cool idea for a package! Your documentation was very extensive and had a lot of examples.

Feedback

  1. The package description can be expanded upon to explain who the target audience is.

  2. Please update the usage example (see screen shot below). It imports a function called get_prime_list_under_n that doesn't exist in num_theory.

Image

  1. pip install num_theory throws an error when run (see screenshot below)

Image

  1. In test_arithmetic_progress.py, please remove the commented out test to reduce clutter and improve readability.

Image

  1. Consider populating the package namespace so that user can make simpler import statements when they use your package. So instead of doing an import statement like: from num_theory.is_prime import is_prime, users can write from num_theory import is_prime

Image

@vrudhgarg
Copy link
Author

@jasonmlee @ThamerD, @dominic-lam @Calista321

Thanks for taking the time to give us feedback—it means a lot to us! I have a few quick questions for you:

  1. We already have information about our package usage by different people at the top of our documentation, but we will add it to our README as well.
  2. You are running pip install from outside the package root. You need to be inside our package folder to run pip install. In fact, without running pip install, you won't be able to import our code files at all.

@vrudhgarg
Copy link
Author

@adrianl726 @ThamerD, @dominic-lam @Calista321

Thanks for taking the time to give us feedback—it means a lot to us!

  1. The decision to choose "Number Theory" in the README was more of a subjective choice for us. We wanted it to be "Number Theory," but we kept the function name short, like num-theory.
  2. We do have some examples of how you can use number-theory in our README file.
Image
  1. As I mentioned to Jason, we have our target audience documented, but we will add that to the README as well.
  2. Regarding your fourth point, yes, that was not part of the milestone, so we did not add more badges. We are planning to incorporate that this week.

@YibinLong
Copy link

YibinLong commented Jan 31, 2025

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • A statement of need clearly stating problems the software is designed to solve and its target audience in README.
  • Installation instructions: for the development version of the package and any non-standard dependencies in README.
  • Vignette(s) demonstrating major functionality that runs successfully locally.
  • Function Documentation: for all user-facing functions.
  • Examples for all user-facing functions.
  • Community guidelines including contribution guidelines in the README or CONTRIBUTING.
  • Metadata including author(s), author e-mail(s), a url, and any other relevant metadata e.g., in a pyproject.toml file or elsewhere.

Readme file requirements
The package meets the readme requirements below:

  • Package has a README.md file in the root directory.

The README should include, from top to bottom:

  • The package name
  • Badges for:
    • Continuous integration and test coverage,
    • Docs building (if you have a documentation website),
    • A repostatus.org badge,
    • Python versions supported,
    • Current package version (on PyPI / Conda).

NOTE: If the README has many more badges, you might want to consider using a table for badges: see this example. Such a table should be more wide than high. (Note that the a badge for pyOpenSci peer-review will be provided upon acceptance.)

  • Short description of package goals.
  • Package installation instructions
  • Any additional setup required to use the package (authentication tokens, etc.)
  • Descriptive links to all vignettes. If the package is small, there may only be a need for one vignette which could be placed in the README.md file.
    • Brief demonstration of package usage (as it makes sense - links to vignettes could also suffice here if package description is clear)
  • Link to your documentation website.
  • If applicable, how the package compares to other similar packages and/or how it relates to other packages in the scientific ecosystem.
  • Citation information

Usability

Reviewers are encouraged to submit suggestions (or pull requests) that will improve the usability of the package as a whole.
Package structure should follow general community best-practices. In general please consider whether:

  • Package documentation is clear and easy to find and use.
  • The need for the package is clear
  • All functions have documentation and associated examples for use
  • The package is easy to install

Functionality

  • Installation: Installation succeeds as documented.
  • Functionality: Any functional claims of the software been confirmed.
  • Performance: Any performance claims of the software been confirmed.
  • Automated tests:
    • All tests pass on the reviewer's local machine for the package version submitted by the author. Ideally this should be a tagged version making it easy for reviewers to install.
    • Tests cover essential functions of the package and a reasonable range of inputs and conditions.
  • Continuous Integration: Has continuous integration setup (We suggest using Github actions but any CI platform is acceptable for review)
  • Packaging guidelines: The package conforms to the pyOpenSci packaging guidelines.
    A few notable highlights to look at:
    • Package supports modern versions of Python and not End of life versions.
    • Code format is standard throughout package and follows PEP 8 guidelines (CI tests for linting pass)

For packages also submitting to JOSS

Note: Be sure to check this carefully, as JOSS's submission requirements and scope differ from pyOpenSci's in terms of what types of packages are accepted.

The package contains a paper.md matching JOSS's requirements with:

  • A short summary describing the high-level functionality of the software
  • Authors: A list of authors with their affiliations
  • A statement of need clearly stating problems the software is designed to solve and its target audience.
  • References: With DOIs for all those that have one (e.g. papers, datasets, software).

Final approval (post-review)

  • The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing: 1.5 hours


Review Comments

Really good work everyone! A few comments:

  1. In the "Comparison with Other Libraries" section, I was wondering if it would be possible to quantify the speed of the libraries. I'm having trouble seeing the difference between Moderate vs Fast vs Very Fast.
  2. Maybe a small introduction on Project Euler questions? I'm not too familiar with them, but it seems useful for it!
  3. It might be helpful to still put an example in "See examples above" in the Key Functions section for arithmetic_progression. I'm not quite sure what arithmetic progression, so it'd help to have a direct example.
  4. I'm a bit confused by what you mean by "API design" - do you mean how the functions are actually called? Maybe just a slight clarification on that would be good!
  5. Maybe mentioning which python versions are supported - I had to dig a bit into the pyproject.toml file to see that python versions >= 3.9 are allowed!

Overall, great work! Clearly a lot of care was taken to make the project.

@ThamerD
Copy link

ThamerD commented Feb 2, 2025

Thank you @YibinLong for your detailed review and your kind words! We have made various improvements to our package thanks to your feedback including:

  1. Approximately quantified speeds of similar packages to improve comparison. improve README according to peer review num_theory#78
  2. Added a description for Project Euler, and clarified the need that our package fulfills. UBC-MDS/num_theory@f8dcb06
  3. Added real life applications of our arithmetic progression function to clarify its use cases. https://github.com/UBC-MDS/num_theory/pull/76/files
  4. Clarified our meaning by "API design" in our README. improve README according to peer review num_theory#78
  5. Added badges to clarify Python version among other useful pieces of info. Peer review dhruv fix num_theory#76
  6. Ensured that our package and documentation included citations wherever applicable. UBC-MDS/num_theory@9a4eddc

We hope that with these changes our package will receive your approval. Feel free to share any additional feedback.
Thanks again!

@ThamerD
Copy link

ThamerD commented Feb 2, 2025

Thank you @adrianl726 for your review! We greatly appreciate how early you submitted it as it allowed us more time to create the necessary improvements including:

  1. Clarified the need that our package fulfills. UBC-MDS/num_theory@f8dcb06
  2. Added badges to provide information on the status of our project, documentation, python version, among other useful pieces of information. Peer review dhruv fix num_theory#76
  3. Ensured that the title of our README exactly matched our package name to avoid confusion. Peer review dhruv fix num_theory#76
  4. Added a description for Project Euler problems. UBC-MDS/num_theory@f8dcb06
  5. Improved the intro section to better clarify our package's functionalities and its intended use cases and audience. UBC-MDS/num_theory@f8dcb06

We hope that these changes meet your expectations and that you will provide us with your approval. Feel free to share any additional feedback.

We appreciate your words of encouragement and wish you the best in your own project!

@ThamerD
Copy link

ThamerD commented Feb 2, 2025

Thank you @jasonmlee for your extensive testing and review! we especially appreciate the detailed screenshots and suggestions you included which enabled us to make the following improvements:

  1. Added a statement of need in the introduction that describes the need our package fulfills and its target audience. UBC-MDS/num_theory@f8dcb06
  2. Added badge for CI/CD, repostatus, Python version, package version, and readthedocs status (+ link to vignette). Peer review dhruv fix num_theory#76 and UBC-MDS/num_theory@02b6e3b
  3. Ensured that our package and documentation included citations wherever applicable. UBC-MDS/num_theory@9a4eddc
  4. Ensured our installation worked consistently (no commit as the fix was done on https://pypi.org/).
  5. Fixed the typo that referenced a function by its old name. UBC-MDS/num_theory@ff278a1
  6. Removed clutter from our test functions. UBC-MDS/num_theory@135f7aa
  7. Populated package namespace to simplify import process. UBC-MDS/num_theory@5254286

We hope that these fixes satisfy your expectations and that you will provide us with your approval. Feel free to share any additional feedback.

Special thanks for your extra suggestions and your explanation on how to implement them! I was wondering how to simplify the import statement and your review saved me a ton of time. Final thank you for submitting your review so early <3

@YibinLong
Copy link

Yeah it looks great! Approving this package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants