-
Notifications
You must be signed in to change notification settings - Fork 1.3k
8620 ModuleNotFoundError: No module named \'onnxscript\' in test-py3x (3.11) pipeline #8621
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: dev
Are you sure you want to change the base?
Conversation
Signed-off-by: R. Garcia-Dias <[email protected]>
Signed-off-by: R. Garcia-Dias <[email protected]>
Signed-off-by: R. Garcia-Dias <[email protected]>
WalkthroughUpdated requirements-dev.txt: changed the onnx dependency from Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
…script-in-test-py3x-311-pipeline
I, R. Garcia-Dias <[email protected]>, hereby add my Signed-off-by to this commit: ba56a6d Signed-off-by: R. Garcia-Dias <[email protected]>
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)
requirements-dev.txt (1)
54-54: Add clarification: why this constraint exists and that it's temporary.The ONNX version constraint lacks inline documentation. Without context, future maintainers won't know this is a workaround for a Python 3.11-specific ONNX 1.19.1 regression or when/how to reassess.
Apply this diff to add documentation:
+# https://github.com/Project-MONAI/MONAI/issues/8620 +# Temporary: ONNX 1.19.1+ breaks Python 3.11 (onnxscript missing). Revisit when ONNX fixes or upgrades. onnx>=1.13.0, <1.19.1; python_version == '3.11'Confirm that Python 3.9, 3.10, and 3.12+ (if tested) are unaffected by ONNX 1.19.1 issues and should not also be constrained. If ONNX 1.19.1 breaks all versions, the constraint should be broadened beyond Python 3.11 only.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
requirements-dev.txt(1 hunks)
⏰ 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). (19)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: build-docs
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: packaging
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-os (macOS-latest)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-py3 (3.9)
🔇 Additional comments (1)
requirements-dev.txt (1)
54-54: Verify the reason for pinning<1.19.1in your environment.ONNX 1.19.1 supports Python 3.11 (Requires-Python >=3.9) and does not add onnxscript as a runtime dependency. The constraint syntax is valid, but the search results don't clarify why this specific upper bound was chosen. Confirm whether later versions (1.19.2, 1.20.x) have documented incompatibilities with your codebase, or if the boundary is conservative pinning.
requirements-dev.txt
Outdated
| optuna | ||
| git+https://github.com/Project-MONAI/MetricsReloaded@monai-support#egg=MetricsReloaded | ||
| onnx>=1.13.0 | ||
| onnx>=1.13.0, <1.19.1; python_version == '3.11' |
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.
I think the python version here will make us only install onnx under this version, does this expected?
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.
Indeed, sorry @KumoLiu.
I meant to add another line for the other Python versions, but I got into the rabbit hole of understanding why it would be failing just to 3.11 and lost track of it.
There are a lot of conditional package installations and building in their library that depend on the version. So it's a bit confusing.
I think we can keep up to 1.19.0 for all Python versions until these problems are fixed. What do you think?
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.
Sure, let's try to run the tests. And come back to this issue later.
Signed-off-by: R. Garcia-Dias <[email protected]>
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
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (1)
requirements-dev.txt(1 hunks)
⏰ 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). (19)
- GitHub Check: flake8-py3 (codeformat)
- GitHub Check: build-docs
- GitHub Check: packaging
- GitHub Check: quick-py3 (windows-latest)
- GitHub Check: quick-py3 (ubuntu-latest)
- GitHub Check: flake8-py3 (pytype)
- GitHub Check: quick-py3 (macOS-latest)
- GitHub Check: flake8-py3 (mypy)
- GitHub Check: min-dep-pytorch (2.8.0)
- GitHub Check: min-dep-pytorch (2.7.1)
- GitHub Check: min-dep-os (ubuntu-latest)
- GitHub Check: min-dep-pytorch (2.6.0)
- GitHub Check: min-dep-py3 (3.12)
- GitHub Check: min-dep-pytorch (2.5.1)
- GitHub Check: min-dep-py3 (3.10)
- GitHub Check: min-dep-py3 (3.11)
- GitHub Check: min-dep-os (windows-latest)
- GitHub Check: min-dep-py3 (3.9)
- GitHub Check: min-dep-os (macOS-latest)
|
/build |
Fixes #8620
Description
On October 8th, there was a new release of the ONNX library (1.19.1).
Looking into previous versions of our CICD, which run successfully, the errors coincide with that date. For example, this one was ok with onnx 1.19.0.
This issue in the ONNX project suggests that there were some recent breaking changes. This is not exactly our issue, but it suggests that there may be something similar going on.
I have added the condition
<1.19.1; python == 3.11to prevent updates on this Python version, since our original issue referred to Python 3.11.It may be that this affects other versions. Let's see if the pipelines execute successfully.
Types of changes
./runtests.sh -f -u --net --coverage../runtests.sh --quick --unittests --disttests.make htmlcommand in thedocs/folder.