-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add no-optionals and full-optionals test runs #8967
Conversation
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the the following people are requested to review this:
|
This restructures the CI slightly to perform a complete "no-optionals" run, and a complete "all optionals" run (for the optionals that are readily accessible as Python packages, without complex additional setup). Previously, we only did a partial test with some of the oldest optional components, which could have allowed for behaviour to accidentally require optional components without us noticing. This splits the `requirements-dev.txt` file into two; lines that remain in the file are what is actually _required_ to run the test suite, run the style checks, and do the documentation build. The rest (and everything that was missing) is added to a new `requirements-optional.txt` file, which can be used to pull in (almost) all of the packages that Terra can use to provide additional / accelerated functionality. Several tests needed to gain additional skips to account for this change. There is a good chance that some tests are missing skips for some libraries that are not the first point of failure, but it's hard to test explicitly for these in one go.
0a14c66
to
a66534b
Compare
Pull Request Test Coverage Report for Build 5600511995
💛 - Coveralls |
Tests should pass after #8973. |
Blocked by #9021. |
Ready for review. |
# Conflicts: # .azure/test-linux.yml # .azure/tutorials-linux.yml # azure-pipelines.yml # requirements-dev.txt # test/python/dagcircuit/test_dagcircuit.py
1791636
to
69a6613
Compare
The failure on Windows is due to #10415. |
With #10441 merged, this is unblocked. |
Will re-review tomorrow. Let's make sure to land this before #10443. |
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.
Thanks! Great cleanup.
Feel free to merge without applying my changes - they're somewhat nitty
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 LGTM, I'm a still mildly concerned about the upper version runs not including the optionals as I just know we've had issues with some optional dependencies lagging on python version support. The nightly jobs provide the coverage, but we've been bad historically about catching failures in the nightly runs. That being said this is a good cleanup and restructuring of how we deal with the optionals and testing. We can always revisit the exact testing matrix and strategy in a followup pr if we have issues for 3.12.
* Add no-optionals and full-optionals test runs This restructures the CI slightly to perform a complete "no-optionals" run, and a complete "all optionals" run (for the optionals that are readily accessible as Python packages, without complex additional setup). Previously, we only did a partial test with some of the oldest optional components, which could have allowed for behaviour to accidentally require optional components without us noticing. This splits the `requirements-dev.txt` file into two; lines that remain in the file are what is actually _required_ to run the test suite, run the style checks, and do the documentation build. The rest (and everything that was missing) is added to a new `requirements-optional.txt` file, which can be used to pull in (almost) all of the packages that Terra can use to provide additional / accelerated functionality. Several tests needed to gain additional skips to account for this change. There is a good chance that some tests are missing skips for some libraries that are not the first point of failure, but it's hard to test explicitly for these in one go. * Fix typo in coverage workflow * Try relaxing ipython constraints * Squash newly exposed lint failures * Fix typo in tutorials pipeline * Update the 'also update' comments * Remove unneeded qiskit-toqm dependency * Section requirements-optional.txt * Test all optionals on min not max Python version Optionals are generally more likely to have been made available on the older Pythons, and some may take excessively long to provide wheels for the latest versions of Python. * Add missing test skip * Fix optional call * Use correct boolean syntax * Fix tests relying on Jupyter * Install ipykernel in tutorials * Remove HAS_PDFLATEX skip from quantum_info tests For simple LaTeX tests, IPython/Jupyter can handle the compilation internally using MathJax, and doesn't actually need a `pdflatex` installation. We only need that when we're depending on LaTeX libraries that are beyond what MathJax can handle natively. * Include additional tutorials dependencies * Install all of Terra's optionals in tutorial run * Do not install all optionals in docs build * Use class-level skips where appropriate * Do not install ibmq-provider in tutorials run * Include matplotlib in docs requirements * Remove unnecessary whitespace * Split long pip line * Only install graphviz when installOptionals Co-authored-by: Eric Arellano <[email protected]> * Install visualization extras in docs build * Don't `--upgrade` when installing optionals This is to prevent any optionals that have a dependency on Terra from potentially upgrading it to some PyPI version during their installation. This shouldn't happen with the current development model of having only one supported stable branch and the main branch, but the `-U` is unnecessary, and not having it is safer for the future. * Update secondary installation of Terra in docs build * Install all optionals during the docs build I yoyo-ed on this, not wanting it to be too onerous to build the documentation, but in the end, we need to have the optional features installed in order to build most of the documentation of those features, and some unrelated parts of the documentation may use these features to enhance their own output. * Fix test setup job * Remove duplication of matplotlib in requirements files * Update image test installation command * Restore editable build * Move `pip check` to `pip` section * Remove redundant "post-install" description * Expand comment on first-stage choices * pytohn lol --------- Co-authored-by: Eric Arellano <[email protected]> (cherry picked from commit 0438a4b)
* Add no-optionals and full-optionals test runs This restructures the CI slightly to perform a complete "no-optionals" run, and a complete "all optionals" run (for the optionals that are readily accessible as Python packages, without complex additional setup). Previously, we only did a partial test with some of the oldest optional components, which could have allowed for behaviour to accidentally require optional components without us noticing. This splits the `requirements-dev.txt` file into two; lines that remain in the file are what is actually _required_ to run the test suite, run the style checks, and do the documentation build. The rest (and everything that was missing) is added to a new `requirements-optional.txt` file, which can be used to pull in (almost) all of the packages that Terra can use to provide additional / accelerated functionality. Several tests needed to gain additional skips to account for this change. There is a good chance that some tests are missing skips for some libraries that are not the first point of failure, but it's hard to test explicitly for these in one go. * Fix typo in coverage workflow * Try relaxing ipython constraints * Squash newly exposed lint failures * Fix typo in tutorials pipeline * Update the 'also update' comments * Remove unneeded qiskit-toqm dependency * Section requirements-optional.txt * Test all optionals on min not max Python version Optionals are generally more likely to have been made available on the older Pythons, and some may take excessively long to provide wheels for the latest versions of Python. * Add missing test skip * Fix optional call * Use correct boolean syntax * Fix tests relying on Jupyter * Install ipykernel in tutorials * Remove HAS_PDFLATEX skip from quantum_info tests For simple LaTeX tests, IPython/Jupyter can handle the compilation internally using MathJax, and doesn't actually need a `pdflatex` installation. We only need that when we're depending on LaTeX libraries that are beyond what MathJax can handle natively. * Include additional tutorials dependencies * Install all of Terra's optionals in tutorial run * Do not install all optionals in docs build * Use class-level skips where appropriate * Do not install ibmq-provider in tutorials run * Include matplotlib in docs requirements * Remove unnecessary whitespace * Split long pip line * Only install graphviz when installOptionals Co-authored-by: Eric Arellano <[email protected]> * Install visualization extras in docs build * Don't `--upgrade` when installing optionals This is to prevent any optionals that have a dependency on Terra from potentially upgrading it to some PyPI version during their installation. This shouldn't happen with the current development model of having only one supported stable branch and the main branch, but the `-U` is unnecessary, and not having it is safer for the future. * Update secondary installation of Terra in docs build * Install all optionals during the docs build I yoyo-ed on this, not wanting it to be too onerous to build the documentation, but in the end, we need to have the optional features installed in order to build most of the documentation of those features, and some unrelated parts of the documentation may use these features to enhance their own output. * Fix test setup job * Remove duplication of matplotlib in requirements files * Update image test installation command * Restore editable build * Move `pip check` to `pip` section * Remove redundant "post-install" description * Expand comment on first-stage choices * pytohn lol --------- Co-authored-by: Eric Arellano <[email protected]> (cherry picked from commit 0438a4b) Co-authored-by: Jake Lishman <[email protected]>
* Add no-optionals and full-optionals test runs This restructures the CI slightly to perform a complete "no-optionals" run, and a complete "all optionals" run (for the optionals that are readily accessible as Python packages, without complex additional setup). Previously, we only did a partial test with some of the oldest optional components, which could have allowed for behaviour to accidentally require optional components without us noticing. This splits the `requirements-dev.txt` file into two; lines that remain in the file are what is actually _required_ to run the test suite, run the style checks, and do the documentation build. The rest (and everything that was missing) is added to a new `requirements-optional.txt` file, which can be used to pull in (almost) all of the packages that Terra can use to provide additional / accelerated functionality. Several tests needed to gain additional skips to account for this change. There is a good chance that some tests are missing skips for some libraries that are not the first point of failure, but it's hard to test explicitly for these in one go. * Fix typo in coverage workflow * Try relaxing ipython constraints * Squash newly exposed lint failures * Fix typo in tutorials pipeline * Update the 'also update' comments * Remove unneeded qiskit-toqm dependency * Section requirements-optional.txt * Test all optionals on min not max Python version Optionals are generally more likely to have been made available on the older Pythons, and some may take excessively long to provide wheels for the latest versions of Python. * Add missing test skip * Fix optional call * Use correct boolean syntax * Fix tests relying on Jupyter * Install ipykernel in tutorials * Remove HAS_PDFLATEX skip from quantum_info tests For simple LaTeX tests, IPython/Jupyter can handle the compilation internally using MathJax, and doesn't actually need a `pdflatex` installation. We only need that when we're depending on LaTeX libraries that are beyond what MathJax can handle natively. * Include additional tutorials dependencies * Install all of Terra's optionals in tutorial run * Do not install all optionals in docs build * Use class-level skips where appropriate * Do not install ibmq-provider in tutorials run * Include matplotlib in docs requirements * Remove unnecessary whitespace * Split long pip line * Only install graphviz when installOptionals Co-authored-by: Eric Arellano <[email protected]> * Install visualization extras in docs build * Don't `--upgrade` when installing optionals This is to prevent any optionals that have a dependency on Terra from potentially upgrading it to some PyPI version during their installation. This shouldn't happen with the current development model of having only one supported stable branch and the main branch, but the `-U` is unnecessary, and not having it is safer for the future. * Update secondary installation of Terra in docs build * Install all optionals during the docs build I yoyo-ed on this, not wanting it to be too onerous to build the documentation, but in the end, we need to have the optional features installed in order to build most of the documentation of those features, and some unrelated parts of the documentation may use these features to enhance their own output. * Fix test setup job * Remove duplication of matplotlib in requirements files * Update image test installation command * Restore editable build * Move `pip check` to `pip` section * Remove redundant "post-install" description * Expand comment on first-stage choices * pytohn lol --------- Co-authored-by: Eric Arellano <[email protected]>
Summary
This restructures the CI slightly to perform a complete "no-optionals" run, and a complete "all optionals" run (for the optionals that are readily accessible as Python packages, without complex additional setup). Previously, we only did a partial test with some of the oldest optional components, which could have allowed for behaviour to accidentally require optional components without us noticing.
This splits the
requirements-dev.txt
file into two; lines that remain in the file are what is actually required to run the test suite, run the style checks, and do the documentation build. The rest (and everything that was missing) is added to a newrequirements-optional.txt
file, which can be used to pull in (almost) all of the packages that Terra can use to provide additional / accelerated functionality.Several tests needed to gain additional skips to account for this change. There is a good chance that some tests are missing skips for some libraries that are not the first point of failure, but it's hard to test explicitly for these in one go.
Details and comments