Bump min python version to 3.10 and update metadata #318
Conversation
WalkthroughThe pull request introduces updates to the CI workflow and the Changes
Assessment against linked issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
setup.py (2)
55-55: Approve numpy dependency update, but consider version rangeRemoving the upper version limit for numpy allows for compatibility with future releases, which aligns with the PR objectives. However, this approach may introduce potential compatibility issues if future numpy versions include breaking changes.
Consider using a more conservative approach by specifying a minimum version and allowing for minor updates:
- "numpy", + "numpy>=1.20,<2",This ensures compatibility with recent numpy versions while protecting against potential breaking changes in major releases.
75-75: Approve Python version update and suggest classifier updateThe change to require Python 3.10 or higher aligns with the PR objectives and practices of related libraries. This is a breaking change that drops support for Python 3.9.
To maintain consistency, please update the classifiers to reflect the new minimum Python version:
classifiers=[ "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", # ... other classifiers ... ],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- .github/workflows/ci.yml (1 hunks)
- setup.py (3 hunks)
🔇 Additional comments (2)
setup.py (2)
11-11: Approve date update, but query future date usageThe date has been appropriately updated to reflect the latest changes. However, both the old and new dates are set in the future.
Could you clarify why future dates are being used? Typically, the
__date__field should reflect the actual release date, not a planned future date. Consider using the current date or leaving it blank until the actual release.
57-57: Approve pymatgen dependency updateRemoving the upper version limit for pymatgen allows for compatibility with future releases. This change directly addresses the concern raised in issue #316 regarding the necessity of the upper version pin.
This change aligns well with the project's goal of improving compatibility with the latest pymatgen releases. However, be prepared to monitor for any potential compatibility issues that may arise with future pymatgen updates.
| @@ -19,7 +19,7 @@ jobs: | |||
| strategy: | |||
| fail-fast: false | |||
| matrix: | |||
| python-version: ["3.9","3.10","3.11","3.12"] | |||
| python-version: ["3.10","3.11","3.12"] | |||
There was a problem hiding this comment.
💡 Codebase verification
Update Documentation to Reflect Python 3.10+ Requirement
The README currently states that the project is tested using Python 3.9+, which conflicts with the CI workflow where Python 3.9 has been removed from the test matrix. Please update the README to specify that the project requires Python 3.10 or higher to ensure consistency and clarity for users.
🔗 Analysis chain
Approved: Python version matrix update aligns with PR objectives
The removal of Python 3.9 from the test matrix is consistent with the PR's goal of dropping support for Python 3.9 and aligning with pymatgen and other NumFOCUS stack libraries. This change appropriately reflects the new minimum Python version requirement of 3.10.
To ensure this change is properly documented, please run the following script:
If the script doesn't return any results or shows outdated information, please update the project's README or documentation to clearly state the new minimum Python version requirement.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the README or documentation has been updated to reflect the new Python version requirement.
# Test: Search for mentions of Python version requirements
rg --type md --type rst "Python (version|>=|>|3\.(9|10))"
Length of output: 138
setup.py
Outdated
| @@ -5,10 +5,10 @@ | |||
| __copyright__ = ( | |||
| "Copyright Daniel W. Davies, Adam J. Jackson, Keith T. Butler (2019)" | |||
| ) | |||
| __version__ = "2.7" | |||
| __version__ = "2.7.1" | |||
There was a problem hiding this comment.
Consider a major version increment for breaking changes
The version number has been updated from 2.7 to 2.7.1, which typically indicates a patch release. However, the PR description mentions a breaking change (dropping support for Python 3.9). Breaking changes usually warrant a major version increment.
Consider updating the version to 3.0.0 to reflect the breaking change:
-__version__ = "2.7.1"
+__version__ = "3.0.0"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| __version__ = "2.7.1" | |
| __version__ = "3.0.0" |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #318 +/- ##
=======================================
Coverage 76.46% 76.46%
=======================================
Files 26 26
Lines 2265 2265
=======================================
Hits 1732 1732
Misses 533 533 ☔ View full report in Codecov by Sentry. |
Description
Following suit with pymatgen which in turn follows numpy, scipy and the rest of the NumFOCUS stack, this PR drops Python 3.9 support.
Fixes #316
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
N/A
Reviewers
Checklist:
Summary by CodeRabbit
numpyandpymatgendependencies to ensure access to the latest features.