Drop Python 3.9 support #178
Conversation
|
Warning Rate limit exceeded@AntObi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 49 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe pull request introduces updates to the CI workflow and project configuration for the ElementEmbeddings project. Key changes include the transition from Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant CI as GitHub Actions
participant UV as UV Package Manager
participant Python as Python Interpreter
Dev->>CI: Trigger Workflow
CI->>Python: Select Python Version (3.10-3.13)
CI->>UV: Upgrade UV
UV->>CI: Install Dependencies
CI->>UV: Check Package Versions
UV->>CI: Return Version Information
Poem
Finishing Touches
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
🧹 Nitpick comments (3)
.github/workflows/ci.yml (1)
37-39: Consider adding version output to package checksThe package version checks could be more informative by capturing the actual versions.
- uv pip show -V elementembeddings - uv pip show -V pytest - uv pip show -V matplotlib + uv pip show elementembeddings | grep -E "^Version|^Name" + uv pip show pytest | grep -E "^Version|^Name" + uv pip show matplotlib | grep -E "^Version|^Name"pyproject.toml (2)
3-3: Consider using newer setuptools versionThe current minimum version might be outdated. Recent versions have important fixes and improvements.
-requires = ["setuptools>=65.0.0"] # PEP 508 specifications. +requires = ["setuptools>=68.0.0"] # PEP 508 specifications.
44-56: Consider adding pytest-xdist for parallel testingThe dev dependencies look comprehensive, but adding pytest-xdist could speed up the test execution in CI.
dev = [ "pre-commit", "black", "isort", "pytest", "pytest-subtests", + "pytest-xdist", "nbqa",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ci.yml(2 hunks)pyproject.toml(1 hunks)setup.py(0 hunks)
💤 Files with no reviewable changes (1)
- setup.py
🧰 Additional context used
🪛 GitHub Actions: ElementEmbeddings CI
.github/workflows/ci.yml
[warning] 20-20: Formatting issue: Missing space after comma in python-version array
🔇 Additional comments (3)
.github/workflows/ci.yml (1)
33-34: Verify uv package manager compatibilityThe switch to using
uvpackage manager is a significant change. While it can improve installation speed, we should ensure it's compatible across all platforms.Run this script to check uv compatibility with the project's dependencies:
✅ Verification successful
UV package manager is compatible with the project
The project uses standard dependency specifications and platform-independent packages from PyPI, making it fully compatible with the UV package manager. The switch to UV is safe as it supports all required features including editable installs and PEP 508 version specifiers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify uv compatibility with project dependencies # Test: Check if uv can resolve all dependencies without conflicts # Get list of dependencies from pyproject.toml rg -A 1 '^\[project\]' pyproject.toml -B 1 | rg 'dependencies'Length of output: 62
Script:
#!/bin/bash # Find Python dependency management files echo "=== Searching for dependency files ===" fd -e toml -e txt -e ini . | grep -iE 'pyproject|requirements|setup|pip' echo -e "\n=== Examining pyproject.toml if exists ===" fd pyproject.toml -x cat {} echo -e "\n=== Examining requirements files if exist ===" fd requirements -e txt -x cat {} echo -e "\n=== Checking for platform-specific configurations ===" rg -i "platform|system|windows|linux|darwin" -g "pyproject.toml" -g "requirements*.txt" -g "setup.{py,cfg}"Length of output: 8944
pyproject.toml (2)
69-71:⚠️ Potential issueUpdate semantic_release configuration
The version_variable still references setup.py which has been removed.
-version_variable = "setup.py:__version__" +version_variable = "pyproject.toml:project.version"Likely invalid or redundant comment.
29-41: Review dependency version constraintsThe dependency specifications need attention:
- Consider adding upper bounds for more dependencies to prevent breaking changes
- Verify if llvmlite>=0.40 is sufficient for umap and numba compatibility
Run this script to check for potential version conflicts:
.github/workflows/ci.yml
Outdated
| @@ -17,7 +17,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","3.13"] | |||
There was a problem hiding this comment.
Fix formatting: Add space after comma in python-version array
Add a space after the comma before "3.13" to maintain consistent formatting.
- python-version: ["3.10", "3.11", "3.12","3.13"]
+ python-version: ["3.10", "3.11", "3.12", "3.13"]📝 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.
| python-version: ["3.10", "3.11", "3.12","3.13"] | |
| python-version: ["3.10", "3.11", "3.12", "3.13"] |
🧰 Tools
🪛 GitHub Actions: ElementEmbeddings CI
[warning] 20-20: Formatting issue: Missing space after comma in python-version array
| version = "0.6.1" | ||
| description = "Element Embeddings" | ||
| readme = "README.md" | ||
| requires-python = ">=3.10" |
There was a problem hiding this comment.
💡 Codebase verification
Dependencies not ready for Python 3.13
While the project declares Python 3.13 support, several key dependencies don't explicitly support Python 3.13 yet:
- pymatgen (up to 3.12)
- seaborn (up to 3.12)
- umap-learn (up to 3.12)
- llvmlite (up to 3.12)
- adjustText (no version info)
- openTSNE (generic Python 3)
Consider removing Python 3.13 from classifiers until these dependencies add support for it.
🔗 Analysis chain
Verify Python version compatibility
The Python version requirement and classifiers align with dropping Python 3.9 support. However, ensure all dependencies support Python 3.13.
Run this script to check dependency compatibility with Python 3.13:
Also applies to: 16-21
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check dependency compatibility with Python 3.13
# Test: Extract dependencies and check their Python version support
# Get all dependencies
rg '^ "[^"]+[>=<]+[0-9.]+' pyproject.toml | while read -r dep; do
pkg=$(echo "$dep" | sed 's/^ "\([^"]*\)[>=<].*/\1/')
echo "Checking $pkg..."
curl -s "https://pypi.org/pypi/$pkg/json" | jq -r '.info.classifiers[] | select(contains("Programming Language :: Python"))'
done
Length of output: 3305
Script:
#!/bin/bash
# Description: Check dependency compatibility with Python 3.13 (fixed version)
# Get all dependencies including optional ones
rg '^ "[^"]+"' pyproject.toml | sed 's/^ "\([^"]*\).*/\1/' | while read -r pkg; do
# Skip empty lines and section headers
if [[ -z "$pkg" || "$pkg" == "["* ]]; then
continue
fi
echo "Checking $pkg..."
curl -s "https://pypi.org/pypi/${pkg%,}/json" | jq -r '.info.classifiers[] | select(contains("Programming Language :: Python :: 3"))'
done
Length of output: 32163
Script:
#!/bin/bash
# Description: Check dependency compatibility with Python 3.13 (improved version)
# Get dependencies section only
echo "Main dependencies:"
rg '^\[project\.dependencies\]' -A 50 pyproject.toml | while read -r line; do
if [[ $line == "["* && $line != "[project.dependencies]" ]]; then
break
fi
if [[ $line =~ ^\"([^\"]+)\" ]]; then
pkg="${BASH_REMATCH[1]}"
echo "Checking $pkg..."
curl -s "https://pypi.org/pypi/$pkg/json" | jq -r '.info.classifiers[] | select(contains("Programming Language :: Python :: 3"))'
echo "---"
fi
done
Length of output: 116
Script:
#!/bin/bash
# First, let's see the content structure
echo "File content:"
cat pyproject.toml
echo -e "\n=== Now checking dependencies ===\n"
# Extract dependencies using a simpler pattern
cat pyproject.toml | awk '/^dependencies = \[/,/\]/' | grep '"' | tr -d '", ' | while read -r pkg; do
if [[ $pkg == *"="* ]]; then
pkg=${pkg%%=*}
fi
if [[ -n $pkg ]]; then
echo "Checking $pkg..."
curl -s "https://pypi.org/pypi/$pkg/json" | jq -r '.info.classifiers[] | select(contains("Programming Language :: Python :: 3"))'
echo "---"
fi
done
Length of output: 11297
Script:
#!/bin/bash
# Check main dependencies for Python 3.13 support
echo "Checking core dependencies for Python 3.13 support:"
for pkg in numpy scipy pymatgen seaborn matplotlib scikit-learn umap-learn adjustText openTSNE typing-extensions llvmlite; do
echo "=== Checking $pkg ==="
curl -s "https://pypi.org/pypi/$pkg/json" | jq -r '.info.classifiers[] | select(contains("Programming Language :: Python :: 3"))'
echo "---"
done
Length of output: 5712
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/elementembeddings/core.py (1)
Line range hint
170-170: Consider centralizing Python version compatibility handling.The current changes add
strict=Falseto multiplezipcalls across different files. Consider:
- Creating a utility function that wraps
zipwith version-specific defaults- Adding version compatibility settings in a central configuration
- Documenting the minimum Python version requirements
This would make future version-specific changes easier to manage.
Also applies to: 258-258, 179-179
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/ci.yml(2 hunks)src/elementembeddings/_base.py(1 hunks)src/elementembeddings/core.py(1 hunks)src/elementembeddings/plotter.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
🔇 Additional comments (3)
src/elementembeddings/plotter.py (1)
179-179: LGTM! Proactive compatibility fix for Python 3.13.The addition of
strict=Falsemaintains the current behavior whereziphandles mismatched lengths gracefully. This is important for processing species labels that may have different lengths.src/elementembeddings/_base.py (1)
170-170: Verify data integrity in standardization process.While adding
strict=Falsemaintains compatibility with Python 3.13, the standardization process should maintain a 1:1 mapping between keys and embeddings. Consider adding validation to ensure data integrity.Run this script to verify the embeddings integrity:
✅ Verification successful
Data integrity is maintained in standardization process
The implementation guarantees that the number of keys and embeddings remain equal throughout the transformation. The
strict=Falseparameter is safe here as both sequences in the zip operation originate from the same dictionary, and the numpy operations preserve the array length.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that embeddings dictionary keys match the number of embedding arrays # Test: Search for instances where embeddings dictionary is modified # Expect: Equal length checks or similar validation ast-grep --pattern 'embeddings_copy = self.embeddings.copy() $$$ embeddings_array = np.array(list(embeddings_copy.values())) $$$'Length of output: 143
Script:
#!/bin/bash # Search for the standardize method implementation ast-grep --pattern 'def standardise($$$): $$$' # Also search with alternative spelling ast-grep --pattern 'def standardize($$$): $$$' # Backup search using ripgrep for context rg "def standard" -A 10Length of output: 4065
src/elementembeddings/core.py (1)
258-258: LGTM! Maintains consistency with base class.The addition of
strict=Falsealigns with the base class implementation. Consider implementing the same data integrity validation suggested for the base class.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #178 +/- ##
=======================================
Coverage 90.95% 90.95%
=======================================
Files 12 12
Lines 1294 1294
=======================================
Hits 1177 1177
Misses 117 117 ☔ View full report in Codecov by Sentry. |
Dropping Python 3.9 Support
Description
llvmlite>=0.40as a requirement to handleumap/numbadependenciesType of change
How Has This Been Tested?
Test Configuration:
Reviewers
N/A
Checklist
Summary by CodeRabbit
CI/CD Updates
uvfor dependency management and package version checkingProject Configuration
pyproject.tomlsetup.pyin favor of modern project configurationEnhancements
standardisemethod in embedding classes to handle different lengths of input iterables without errors in multiple modules.