Skip to content

Conversation

@garciadias
Copy link
Contributor

Fixes #8620 .

Description

Adds onnxscript as an explicit dependency.

I have tried to find where this onnxscript package was coming from before. For that, I tried all Python versions from 3.9 to 3.12, all versions of onnxruntime and onnx_graphsurgeon, and all versions later than 1.13.0 of onnx.

None of these would include onnxscript.

I suppose that this was a requirement of another library and was removed in some new version.
I don't think it is worth the trouble of further investigating to find which package it was, since we wouldn't want to freeze a package version for this reason. So, instead, I propose we just add onnxscript as a dependency.

Potential issue

I am not sure if this will trigger the running of the ONNX tests in Python < 3.10 and how it will impact those tests.

A few sentences describing the changes proposed in this pull request.

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 20, 2025

Walkthrough

The pull request modifies requirements-dev.txt by removing the upper bound constraint from the onnx dependency (changing from onnx>=1.13.0, <1.19.1 to onnx>=1.13.0) and adding onnxscript as a new dependency. These changes directly address the missing module error encountered during ONNX export tests.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single file modification with straightforward dependency adjustments
  • No code logic changes or structural alterations
  • Homogeneous, repetitive-pattern changes (dependency specification updates)

Areas requiring extra attention:

  • Verify that the removal of the onnx upper bound (<1.19.1) does not introduce incompatibilities with torch or other ONNX-related packages
  • Confirm onnxscript version constraint is appropriate (no explicit version pin specified; consider if a minimum version is needed)

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title references issue #8620 and describes the core problem (ModuleNotFoundError for onnxscript in Python 3.11 pipeline), matching the changeset's solution.
Description check ✅ Passed Description addresses the core issue, explains the investigation rationale, and follows the template structure with appropriate sections and checkboxes marked.
Linked Issues check ✅ Passed PR directly addresses issue #8620 by adding onnxscript as an explicit dependency, resolving the ModuleNotFoundError that blocked ONNX export tests.
Out of Scope Changes check ✅ Passed Changes are limited to requirements-dev.txt: removing upper bound on onnx and adding onnxscript dependency. Both align with fixing the reported issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent 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 701692c and f5a9dda.

📒 Files selected for processing (1)
  • requirements-dev.txt (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • requirements-dev.txt
⏰ 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: min-dep-pytorch (2.5.1)
  • GitHub Check: min-dep-pytorch (2.7.1)
  • GitHub Check: min-dep-pytorch (2.6.0)
  • GitHub Check: min-dep-pytorch (2.8.0)
  • GitHub Check: min-dep-os (windows-latest)
  • GitHub Check: min-dep-os (macOS-latest)
  • GitHub Check: min-dep-py3 (3.12)
  • GitHub Check: min-dep-os (ubuntu-latest)
  • GitHub Check: min-dep-py3 (3.9)
  • GitHub Check: min-dep-py3 (3.11)
  • GitHub Check: min-dep-py3 (3.10)
  • GitHub Check: quick-py3 (ubuntu-latest)
  • GitHub Check: build-docs
  • GitHub Check: flake8-py3 (codeformat)
  • GitHub Check: packaging
  • GitHub Check: quick-py3 (windows-latest)
  • GitHub Check: quick-py3 (macOS-latest)
  • GitHub Check: flake8-py3 (mypy)
  • GitHub Check: flake8-py3 (pytype)

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 and others added 3 commits November 20, 2025 13:56
I, R. Garcia-Dias <[email protected]>, hereby add my Signed-off-by to this commit: 9f4d989

Signed-off-by: R. Garcia-Dias <[email protected]>
…script-in-test-py3x-311-pipeline

Signed-off-by: Rafael Garcia-Dias <[email protected]>
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

1 participant