Skip to content

Conversation

mtreinish
Copy link
Member

Summary

This commit adds a new script that is run in CI to find whether slow
optional imports are in the default qiskit import path. We've recently
had several instances of PRs being pushed that added sympy to the
default import path which makes the overall qiskit import significantly
slower (see #5576 for the most recent occurrence), this script will
catch those situations and report the error.

Details and comments

This commit adds a new script that is run in CI to find whether slow
optional imports are in the default qiskit import path. We've recently
had several instances of PRs being pushed that added sympy to the
default import path which makes the overall qiskit import significantly
slower (see Qiskit#5576 for the most recent occurrence), this script will
catch those siutations and report the error.
@mtreinish mtreinish requested a review from a team as a code owner January 5, 2021 13:14
The azure ci env was using system python instead of the venv where
qiskit-terra was installed for running the new script. This was causing
an error because to work the script imports qiskit to verify nothing
slow is getting imported. This commit fixes this by explicitly using the
venv python instead of the default system python to execute the script.
@mtreinish
Copy link
Member Author

We can add matplotlib to the list after: #5485 is merged

@mergify mergify bot merged commit 48c09fa into Qiskit:master Feb 4, 2021
mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Feb 5, 2021
In Qiskit#5582 we added a script that is used to ensure we don't import
slow optional dependencies in the default 'import qiskit' path. Since
that PR merged we've managed to remove a few more packages from the
default import path (see Qiskit#5619, Qiskit#5485, and Qiskit#5620) which we should add
the same checks to to ensure we don't regress and accidently start
importing them again in the default path. This commit adds all these
packages to the list of imports not allowed as part of 'import qiskit'.
mergify bot pushed a commit that referenced this pull request Feb 5, 2021
In #5582 we added a script that is used to ensure we don't import
slow optional dependencies in the default 'import qiskit' path. Since
that PR merged we've managed to remove a few more packages from the
default import path (see #5619, #5485, and #5620) which we should add
the same checks to to ensure we don't regress and accidently start
importing them again in the default path. This commit adds all these
packages to the list of imports not allowed as part of 'import qiskit'.
@kdk kdk added the Changelog: None Do not include in changelog label Mar 31, 2021
@mtreinish mtreinish deleted the add-test-for-imports branch November 8, 2021 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants