style: enable B905 to prevent issues with zip #1580
Conversation
This PR explicitly requires the input of zip to have the same length. If not, there should be something wrong. Signed-off-by: Jinzhe Zeng <[email protected]>
📝 Walkthrough📝 WalkthroughWalkthroughThe change involves modifications to the Changes
Possibly related PRs
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 (
|
|
I just realize I make the PR a draft until we upgrade the minimal Python version to Python 3.10. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #1580 +/- ##
=======================================
Coverage 49.53% 49.53%
=======================================
Files 83 83
Lines 14849 14849
=======================================
Hits 7356 7356
Misses 7493 7493 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Jinzhe Zeng <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
pyproject.toml (2)
Line range hint
19-20: LGTM! Consider optimizing optional dependencies.The addition of optional dependency groups and the version-specific GromacsWrapper dependency are good improvements to the project structure. They allow for more flexible installation options and better version compatibility.
Consider removing 'dpgui' from the 'test' group since it's already in the 'gui' group. If 'dpgui' is required for testing, you might want to include the 'gui' group in the 'test' group instead:
[project.optional-dependencies] gui = [ "dpgui", ] test = [ "coverage", "pymatgen-analysis-defects<2023.08.22", "gui", ]This change would reduce redundancy and make the dependencies more maintainable.
Also applies to: 46-55
Line range hint
28-28: Update Python version requirementThe PR objective mentions that the 'strict' option for zip() is new in Python 3.10. However, the
requires-pythonfield still allows for Python 3.9+.Consider updating the Python version requirement to 3.10+ to align with the use of the 'strict' option:
-requires-python = ">=3.9" +requires-python = ">=3.10"Also, you may want to update the classifiers to remove Python 3.9 support:
classifiers = [ - "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", "License :: OSI Approved :: GNU Lesser General Public License v3 (LGPLv3)", ]These changes will ensure that the project's stated Python version requirements are consistent with the features being used.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- pyproject.toml (1 hunks)
🔇 Additional comments (1)
pyproject.toml (1)
89-90: LGTM! Verify impact of banning pymatgen imports.The addition of the B905 rule aligns well with the PR objective of preventing issues with zip() usage. The per-file ignore for tests is also a good practice.
Regarding the ban on module-level imports of 'pymatgen', please verify that this change doesn't break existing functionality. Run the following script to check for any remaining module-level imports of pymatgen:
If this script returns any results, those files may need to be updated to comply with the new import restrictions.
Also applies to: 92-94, 101-103
This PR explicitly requires the input of zip to have the same length. If not, there should be something wrong.
Summary by CodeRabbit