Skip to content

Conversation

@garciadias
Copy link
Contributor

@garciadias garciadias commented Nov 4, 2025

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.11 to 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

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

Walkthrough

Updated requirements-dev.txt: changed the onnx dependency from onnx>=1.13.0 to onnx>=1.13.0, <1.19.1. Minor formatting edit in README.md: removed a leading space before an <img ...> tag. No other logic, control-flow, or functional code changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

  • Files to spot-check:
    • requirements-dev.txt (verify dependency spec syntax)
    • README.md (visual/formatting only)

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title directly references issue #8620 and accurately describes the ModuleNotFoundError affecting the Python 3.11 pipeline.
Description check ✅ Passed Description includes issue reference, clear explanation of the problem, rationale for the fix, and completion of the PR template with appropriate checkboxes.
Linked Issues check ✅ Passed PR adds ONNX version constraint (<1.19.1) for Python 3.11 to address the ModuleNotFoundError reported in issue #8620, directly resolving the root cause.
Out of Scope Changes check ✅ Passed Changes are scoped to requirements-dev.txt and a minor README formatting fix; both align with fixing the ONNX compatibility issue for Python 3.11.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@garciadias garciadias marked this pull request as ready for review November 4, 2025 16:46
I, R. Garcia-Dias <[email protected]>, hereby add my Signed-off-by to this commit: ba56a6d

Signed-off-by: R. Garcia-Dias <[email protected]>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between e72145c and 4ce587d.

📒 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.1 in 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.

@garciadias garciadias requested a review from KumoLiu November 4, 2025 17:20
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'
Copy link
Contributor

@KumoLiu KumoLiu Nov 5, 2025

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?

Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 76a8b9a and 1cb46a0.

📒 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)

@garciadias garciadias requested a review from KumoLiu November 5, 2025 08:27
@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 5, 2025

/build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ModuleNotFoundError: No module named \'onnxscript\' in test-py3x (3.11) pipeline

2 participants