-
Notifications
You must be signed in to change notification settings - Fork 104
Modernize dependency pinnings #723
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?
Conversation
📝 WalkthroughWalkthroughUpdated many dependency version constraints across conda environments, recipes, and Python project pyproject.toml files: removed pre-release suffixes (dev0, a0), tightened/pinned several packages (rapids-build-backend, numpy, pytest, cuda-python, numba-cuda), replaced Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-11-25T10:20:49.822ZApplied to files:
🔇 Additional comments (9)
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.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
conda/environments/all_cuda-129_arch-aarch64.yaml(4 hunks)conda/environments/all_cuda-129_arch-x86_64.yaml(4 hunks)conda/environments/all_cuda-130_arch-aarch64.yaml(4 hunks)conda/environments/all_cuda-130_arch-x86_64.yaml(4 hunks)conda/recipes/cuopt-server/conda_build_config.yaml(0 hunks)conda/recipes/cuopt-server/recipe.yaml(1 hunks)conda/recipes/cuopt/recipe.yaml(3 hunks)conda/recipes/mps-parser/recipe.yaml(1 hunks)dependencies.yaml(6 hunks)python/cuopt/cuopt/linear_programming/pyproject.toml(4 hunks)python/cuopt/pyproject.toml(3 hunks)python/cuopt_self_hosted/pyproject.toml(1 hunks)python/cuopt_server/pyproject.toml(3 hunks)python/libcuopt/pyproject.toml(1 hunks)
💤 Files with no reviewable changes (1)
- conda/recipes/cuopt-server/conda_build_config.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to docs/**/*.{rst,md} : Update version numbers and deprecation notices in documentation for breaking API changes; provide migration guidance for deprecated APIs
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to docs/**/*.{rst,md} : Update version numbers and deprecation notices in documentation for breaking API changes; provide migration guidance for deprecated APIs
Applied to files:
dependencies.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.12, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.11, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.13, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.10, arm64, rockylinux8
- GitHub Check: wheel-build-cuopt-mps-parser / 13.0.2, 3.12, amd64, rockylinux8
- GitHub Check: checks / check-style
🔇 Additional comments (23)
conda/recipes/mps-parser/recipe.yaml (2)
48-48: LGTM! Cleaner numpy constraint without pre-release suffix.The removal of the
a0suffix from the upper bound makes the constraint cleaner and more standard. The latest numpy version is 2.3.5, and numpy 3.0 has not been released, so the constraint>=1.23,<3.0remains appropriate and will effectively exclude numpy 3.0 and later when it is eventually released.
45-45: Constraint simplification is sound.Removing the
.dev0suffix from the upper bound is a valid simplification. Both<0.5.0.dev0and<0.5.0effectively exclude version 0.5.0 and later, since dev versions sort before final releases in conda's versioning scheme. The cleaner constraint aligns with standard conda best practices.conda/environments/all_cuda-130_arch-aarch64.yaml (6)
59-59: Constraint is justified—pytest 9.0 includes breaking changes.pytest 9.0 drops support for Python 3.9 and makes PytestRemovedIn9Warning deprecation warnings errors by default. Additionally, breaking changes were introduced to how overlapping arguments and duplicates are handled. The upper bound
pytest<9.0appropriately protects against these incompatibilities.
79-79: The package namenvidia-sphinx-themeis correct. This is the official package available on PyPI, and is described as "A Sphinx theme for NVIDIA projects". The hyphenated form is the standard package naming convention for PyPI; the underscore variant appearing in wheel filenames (e.g.,nvidia_sphinx_theme-0.0.5.post1-py3-none-any.whl) is a standard Python packaging convention and does not indicate a separate package.
19-19: No concerns identified. The constraintcuda-python>=13.0.1,<14.0is valid and compatible with CUDA 13.0 on aarch64 architecture.
47-48: Review version constraint for numba-cuda.The constraint
numba-cuda>=0.22.1,<0.23.0specifies a version that does not appear to exist in the official releases. The latest numba-cuda release as of December 2024 is v0.22.0. Verify that this version constraint is intentional and that the environment should reference an available release, or confirm that v0.22.1 has been released since then.If referencing v0.22.0, numba-cuda is tested with CUDA 13, and Numba 0.60.0 has binary support for NumPy 2.0, though execution-level compatibility is not guaranteed.
49-49: Constrain numpy to compatible minor versions within 2.x.The constraint
numpy>=1.23.5,<3.0allows any numpy 2.x release, but packages like numba have version-specific upper bounds on numpy. Consider usingnumpy>=1.23.5,<2.3or tightening the constraint based on your environment's pinned package versions to avoid compatibility issues.
61-61: Confirm rapids-build-backend version 0.4.0 availability.The constraint
>=0.4.0,<0.5.0should be validated as all current RAPIDS releases use rapids-build-backend 0.4.1. Verify whether version 0.4.0 exists or if the version should be pinned to 0.4.1 for RAPIDS 26.2.python/cuopt/pyproject.toml (1)
7-7: LGTM: Dependency constraints properly modernized.The version constraint updates consistently remove pre-release suffixes (dev0, a0) and align with the broader project-wide dependency modernization, matching the updates in environment files and other pyproject.toml files.
Also applies to: 22-22, 27-29, 48-50
conda/recipes/cuopt/recipe.yaml (1)
65-65: LGTM: Recipe dependencies properly updated.The dependency constraints are correctly updated across both host and run requirements, with appropriate conditional logic for different CUDA major versions (12 vs 13). The changes align with the project-wide modernization effort.
Also applies to: 70-71, 80-81, 93-94
conda/environments/all_cuda-130_arch-x86_64.yaml (1)
19-19: LGTM: CUDA 13.0 environment properly updated.The dependency constraints are consistent with other environment files, appropriately using CUDA 13-specific package versions where needed. The updates align with the modernization effort.
Also applies to: 47-47, 49-49, 59-59, 61-61, 79-79
conda/environments/all_cuda-129_arch-x86_64.yaml (1)
19-19: LGTM: x86_64 environment properly synchronized.The dependency constraints match the aarch64 variant for CUDA 12.9, maintaining consistency across architectures. The updates align with the modernization effort.
Also applies to: 47-47, 49-49, 59-59, 61-61, 79-79
dependencies.yaml (1)
255-255: LGTM: Source dependency definitions properly updated.As the source file for all auto-generated conda environment files, these updates consistently remove pre-release suffixes and modernize version constraints. The changes properly handle CUDA version matrices and will propagate correctly to all generated files.
Also applies to: 327-327, 337-337, 351-351, 366-366, 370-370, 757-757
python/cuopt_self_hosted/pyproject.toml (1)
39-39: LGTM: Pytest constraint aligned with project-wide update.The update from
<8to<9.0is consistent with the pytest version modernization applied across all other test configurations in this PR.conda/environments/all_cuda-129_arch-aarch64.yaml (2)
47-47: Verify numba-cuda 0.22.1 compatibility and test impact of architectural changes.The update from
>=0.19.1,<0.20.0a0to>=0.22.1,<0.23.0involves internal architectural refactoring between versions. While versions 0.21 and 0.22 introduced significant internal changes (including binding mechanism modifications), confirm that these changes do not impact code using the CUDA target at the user API level.
59-59: Verify pytest 8.x compatibility through testing.pytest 8.0 introduced breaking changes to pytest's collection phase, particularly around how filesystem directories and Python packages are collected. Key changes include:
- pytest.Package now only collects files in its own directory; previously it collected recursively
- The default value for junit_family changed to xunit2; if you require the old format, add junit_family=xunit1 to your configuration file
- The TerminalReporter no longer has a writer attribute; plugin authors may use the public functions of the TerminalReporter instead
Ensure the test suite runs successfully with pytest 8.x versions and check for any deprecated patterns or removed APIs in use.
python/cuopt_server/pyproject.toml (3)
7-7: Approve rapids-build-backend constraint modernization.Same modernization as in
python/libcuopt/pyproject.toml. The change is consistent across the project.
29-29: Approve numpy constraint update - now allows 3.0 pre-releases.The change from
<3.0a0to<3.0widens the constraint to permit numpy 3.0 alpha and beta releases while still excluding the final 3.0 release. This aligns with testing against upcoming numpy versions.
51-51: Clarify pytest version constraint limits in pyproject.toml.The constraint
"pytest<9.0"excludes all pytest 9.x versions (including pre-releases), not permits them. The constraint allows pytest 8.x (including development versions) but does not allow pytest 9.0.0a1, 9.0.0rc1, or any 9.x release. If pytest 9.x compatibility testing is intended, the constraint should be updated accordingly. Otherwise, confirm that excluding pytest 9.x aligns with compatibility goals, given that pytest 8.0 introduced breaking changes to the collection phase, particularly around filesystem directories and Python packages.python/cuopt/cuopt/linear_programming/pyproject.toml (3)
7-7: Approve rapids-build-backend constraint modernization.Consistent with the modernization applied across all project files.
22-22: Approve numpy constraint updates in both locations.The numpy constraint is correctly updated in both the
dependenciessection (line 22) and thetool.rapids-build-backend.requiressection (line 85). This consistency ensures that the same numpy versions are allowed during both runtime and build time, permitting numpy 3.0 pre-releases for forward compatibility testing.Also applies to: 85-85
41-41: Approve pytest constraint update.Consistent with the pytest update in
python/cuopt_server/pyproject.toml. The verification script provided in that file's review will cover test compatibility across the project.python/libcuopt/pyproject.toml (1)
7-7: Approve modernization of rapids-build-backend constraint.The change from
<0.4.0dev0to<0.5.0modernizes the version specifier by removing the non-standard.dev0suffix. Under PEP 440, development releases sort before their corresponding final releases, so this constraint correctly allows development versions of 0.5.0 while excluding the 0.5.0 final release, aligning with modern Python packaging practices.
| - psutil>=6.0.0 | ||
| - python | ||
| - uvicorn ${{ uvicorn_version }} | ||
| - uvicorn =0.34 |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
bdice
left a 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.
Some explanatory comments.
| - cuda-nvcc | ||
| - cuda-nvtx-dev | ||
| - cuda-python>=12.9.2,<13.0a0 | ||
| - cuda-python>=12.9.2,<13.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.
See rapidsai/build-planning#144 for the motivation to remove upper bound alpha/dev labels.
Also in #660 which might merge first.
| - zlib | ||
| - pip: | ||
| - nvidia_sphinx_theme | ||
| - nvidia-sphinx-theme |
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.
This package name was incorrect and relied on PyPI's normalization logic to fetch the right thing.
| - pyrsistent | ||
| - pytest-cov | ||
| - pytest<8 | ||
| - pytest<9.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.
We updated the rest of RAPIDS to PyTest 8 a while ago, and version 9 is out now. Let's try to move this pinning forward.
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.
numpy_version and fastapi_version were unused, so I moved the definition of uvicorn_version into the recipe too.
| - rapidsai | ||
| - conda-forge | ||
| dependencies: | ||
| - boost |
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.
We should replace boost with libboost-devel. The boost package has some Python pieces but we should only need C++ development libraries.
| - cxx-compiler | ||
| - cython>=3.0.3 | ||
| - doxygen=1.9.1 | ||
| - exhale |
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 exhale dependency pins sphinx tightly (<5) and we require newer Sphinx versions in most of RAPIDS. This doesn't appear to be used so I removed it.
|
Thank you @bdice for the PR. |
Description
This PR updates some dependency pinnings to better align with RAPIDS.
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.