-
Notifications
You must be signed in to change notification settings - Fork 0
Downgrade minimum numpy to 1.23.5 #61
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
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Poem
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 (
|
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: 0
🧹 Nitpick comments (1)
pyproject.toml (1)
39-39
: Document the rationale for numpy version downgrade.Consider adding a comment in pyproject.toml explaining why numpy>=1.23.5 was chosen to help future maintainers understand this decision.
dependencies = [ - "numpy>=1.23.5", + # Pinned to >=1.23.5 for compatibility with napari docs (see https://github.com/napari/docs/pull/548) + "numpy>=1.23.5", ]< 8000 /div>📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro📒 Files selected for processing (1)
pyproject.toml
(1 hunks)⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Test on macos-latest with Python 3.9
- GitHub Check: Test on windows-latest with Python 3.12
- GitHub Check: Test on windows-latest with Python 3.11
- GitHub Check: Test on windows-latest with Python 3.10
- GitHub Check: Test on windows-latest with Python 3.9
🔇 Additional comments (2)
pyproject.toml (2)
39-39
: Verify numpy version compatibility with supported Python versions.The downgrade to numpy>=1.23.5 needs verification for compatibility with Python 3.9-3.12. Let's check the compatibility matrix:
✅ Verification successful
Version downgrade to numpy>=1.23.5 appears safe
The codebase primarily uses fundamental numpy features (array operations, type definitions) that are stable across versions. The package supports Python >=3.8, which aligns with the project's Python version requirements (3.9-3.12).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check numpy 1.23.5 compatibility with Python versions # Get numpy release date and Python version support curl -s https://pypi.org/pypi/numpy/1.23.5/json | jq -r '.info | {requires_python, version, release_date: .release_date[0:10]}' # Check for any known issues or breaking changes gh api graphql -f query=' { repository(owner: "numpy", name: "numpy") { issues(first: 5, states: OPEN, labels: ["Bug", "Python Support"], searchQuery: "1.23.5 in:title,body") { nodes { title url labels(first: 5) { nodes { name } } } } } }'Length of output: 873
Script:
#!/bin/bash # Search for numpy imports and usage in the codebase echo "=== Numpy imports and usage patterns ===" rg "^import numpy|^from numpy" -l echo -e "\n=== Specific numpy features usage ===" rg "numpy\." -B1 -A1 # Try to find changelog or release notes echo -e "\n=== Checking for local numpy documentation ===" fd "CHANGELOG|RELEASE|NOTES" -iLength of output: 13446
39-39
: Ensure comprehensive testing across numpy versions.Since the project has a test suite (pytest), it would be valuable to verify that all tests pass with numpy 1.23.5, 1.24.x, and 1.25.x to ensure backward compatibility.
Consider adding the following test environments to your CI pipeline:
[tool.pytest.ini_options] # Add numpy version matrix testing required-plugins = ["pytest-matrix"] matrix = [ {"numpy": ["1.23.5", "1.24.0", "1.25.0"]} ]
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/wheels.yml
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Test on macos-latest with Python 3.10
- GitHub Check: Test on windows-latest with Python 3.12
- GitHub Check: Test on windows-latest with Python 3.11
- GitHub Check: Test on windows-latest with Python 3.10
- GitHub Check: Test on windows-latest with Python 3.9
🔇 Additional comments (4)
.github/workflows/wheels.yml (4)
19-25
: LGTM! Comprehensive test matrix configuration.The test matrix provides good coverage across operating systems and Python versions (3.9-3.12).
60-60
: LGTM! Correct dependency chain.The build_wheels job correctly depends on both test and test_min_req jobs, ensuring wheels are only built after all testing passes.
66-66
: LGTM! Good platform coverage for wheels.The OS matrix provides comprehensive coverage, including newer macOS versions.
98-104
: LGTM! Secure PyPI publishing configuration.The upload job is properly configured with trusted publishing and appropriate permissions.
test_min_req: | ||
name: Test minimum requirements | ||
runs-on: ubuntu-22.04 | ||
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: actions/setup-python@v5 | ||
name: Install Python 3.9 | ||
with: | ||
python-version: '3.9' | ||
- name: Install tox | ||
run: pip install tox tox-gh-actions tox-min-req | ||
- name: Test Backend | ||
run: tox | ||
|
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.
💡 Codebase verification
Missing tox configuration for minimum requirements testing
The workflow installs tox-min-req
, but there's no corresponding configuration in tox.ini
to enable minimum requirements testing. This means the job won't effectively test minimum dependencies as intended.
- Add appropriate configuration in
tox.ini
to enable minimum requirements testing with thetox-min-req
plugin - Consider adding a dedicated test environment for minimum requirements testing, e.g.,
[testenv:min-req]
🔗 Analysis chain
LGTM! Well-configured minimum requirements test.
The job is correctly set up to test minimum dependencies using tox-min-req. Using Python 3.9 on Ubuntu 22.04 is appropriate for minimum requirement testing.
To ensure tox is properly configured for minimum requirements testing, run:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check tox configuration for minimum requirements testing
rg -A 5 'min-req|minreq|minimum' tox.ini pyproject.toml
Length of output: 57
Script:
#!/bin/bash
# First, check if tox configuration files exist
fd "tox.ini|pyproject.toml"
# Then, search for any tox or test requirement configurations
rg -i "testenv|deps|requirements|python_version|tox" pyproject.toml tox.ini setup.cfg setup.py
# Finally, search for any min/minimum patterns in configuration files
fd -e ini -e toml -e cfg -e py -e txt | xargs rg -i "min.*req|req.*min|python.*3\.9"
Length of output: 1015
Check if NumPy minimum version could be lowered to resolve napari/docs#548
Summary by CodeRabbit
test_min_req
to validate minimum project requirements in the workflow.