Move dependencies from requirements.txt to an optional packaging extra #638
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Signed-off-by: Keith Kraus <[email protected]>
kkraus14
force-pushed
the
dependency_update
branch
from
906f845 to
23d48a9
Compare
|
/ok to test |
This comment has been minimized.
This comment has been minimized.
…ies as well Signed-off-by: Keith Kraus <[email protected]>
|
/ok to test |
leofang
reviewed
May 15, 2025
leofang
added
enhancement
Co-authored-by: Leo Fang <[email protected]>
rwgk
reviewed
May 15, 2025
| "numpy>=1.21.1", | ||
| "pytest>=6.2.4", | ||
| "pytest-benchmark>=3.4.1", | ||
| "llvmlite" |
There was a problem hiding this comment.
Looks good to me, but just to point out our options:
- This could be left commented out (status quo), with
# Uncomment to generate MINIMAL_NVVMIR_BITCODE_STATIC for test_nvvm.py (see PR #443).
- If we choose to make this an unconditional
testdependency: I'll work on a follow-on PR to simplify the logic incuda_bindings/tests/test_nvvm.py. I believe we can then remove theMINIMAL_NVVMIR_BITCODE_STATICdictionary entirely.
There was a problem hiding this comment.
This is something I wanted to discuss. I could pull it out into a separate optional dependency group of something like test-nvvm or something of the likes. One of my motivations for uncommenting it was to have it start being tested in CI.
There was a problem hiding this comment.
My vote, based on the quick experiment below (pip install llvmlite takes less than 1 second):
- Make
llvmlitean unconditionaltestdependency, as you have it right now. - I'll do the follow-on PR (quickly, should be easy to squeeze in).
0db6015-lcedt.nvidia.com:~ $ python -m venv junk
0db6015-lcedt.nvidia.com:~ $ . junk/bin/activate
(junk) 0db6015-lcedt.nvidia.com:~ $ pip install --upgrade pip
...
Successfully installed pip-25.1.1
(junk) 0db6015-lcedt.nvidia.com:~ $ time pip install llvmlite
Collecting llvmlite
Using cached llvmlite-0.44.0-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (5.0 kB)
Using cached llvmlite-0.44.0-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (42.4 MB)
Installing collected packages: llvmlite
Successfully installed llvmlite-0.44.0
real 0m0.867s
user 0m0.662s
sys 0m0.151s
|
/ok to test |
|
/ok to test |
|
/ok to test |
leofang
reviewed
May 15, 2025
leofang
reviewed
May 15, 2025
leofang
reviewed
May 16, 2025
|
/ok to test |
leofang
approved these changes
May 16, 2025
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Moves packages needed for testing to an optional dependency group and updates CI and docs to use said optional dependency group. Additionally removes some unused dependencies and specifies versions where they weren't previously specified but were required.
Checklist