-
Notifications
You must be signed in to change notification settings - Fork 20
Update packaging #137
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
base: main
Are you sure you want to change the base?
Update packaging #137
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughUpdated Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR modernizes the pyproject.toml configuration to align with current PEP 621 standards. The changes migrate deprecated metadata fields to their modern equivalents and add support for newer Python versions.
Changes:
- Updated flit_core minimum version from 3.2 to 3.12.0
- Migrated deprecated author fields to modern
authorsarray format - Moved homepage URL to new
[project.urls]section - Added Python 3.12 and 3.13 classifiers and updated license specification
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Topic :: Scientific/Engineering :: Chemistry", | ||
| ] | ||
| license = "BSD-3-Clause" | ||
| license-files = ["LICENSE"] |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The license-files field is redundant when using the SPDX license identifier format. When license is specified as a string (SPDX identifier), the license-files field is not needed according to PEP 621. Either use license = {text = \"BSD-3-Clause\"} with license-files, or use just license = \"BSD-3-Clause\" without license-files.
| license-files = ["LICENSE"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does this come from? It's a bad idea and it's not compliant with PEP 639.
See "Use a default value for license-files if not specified" in Appendix: Rejected Ideas of PEP 639.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pyproject.toml`:
- Around line 7-11: The authors array currently contains two separate entries
(one with {name} and one with {email}) which yields separate authors; update the
authors array so the contact is a single table entry containing both keys (e.g.,
combine into one {name = "...", email = "..."} object) so the metadata has one
author with a contact email; edit the authors block in pyproject.toml to replace
the two entries with a single combined table.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml
🔇 Additional comments (1)
pyproject.toml (1)
2-2: No action needed. The flit_core>=3.12.0 version requirement is correct—flit_core 3.12.0 explicitly added support for PEP 639 SPDX license expressions and thelicense-filesfield used in this pyproject.toml (lines 25–26).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| readme = "README.md" | ||
| readme-content-type = "text/markdown" | ||
| classifiers = [ | ||
| "Intended Audience :: Science/Research", |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'License :: OSI Approved :: BSD License' classifier was removed but not replaced with a more specific classifier. Consider adding 'License :: OSI Approved :: BSD License' back to the classifiers list to maintain discoverability on PyPI, as classifiers serve a different purpose than the license field.
| "Intended Audience :: Science/Research", | |
| "Intended Audience :: Science/Research", | |
| "License :: OSI Approved :: BSD License", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your suggestion is not compliant with current Python packaging standards and PyPi is perfectly aware of the license field.
And where does "classifiers serve a different purpose than the license field" come from? Please provide links to authoritative documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PyPI moved to license in 2024:
pypi/warehouse#16620 (comment)
0d5c207 to
62ca287
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pyproject.toml`:
- Line 2: Update the pyproject.toml requires entry to reflect the true minimum
for PEP 639 support or document the reason for the higher pin: either change the
requires line from "flit_core >=3.12.0,<4" to "flit_core >=3.11.0,<4" if you
want the strict minimum that added PEP 639 support, or keep ">=3.12.0,<4" but
add a brief comment near the requires declaration explaining that 3.12.0 was
chosen for additional SPDX AND/OR and license-file handling improvements rather
than being the minimal PEP 639 requirement.
323cf26 to
c862cdb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c862cdb to
4dbf3ce
Compare
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.