Add a lazy loading wrapper class for __qiskit_version__ #5620
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit migrates the __qiskit_version__ module attribute away from using a dict constructed at import time to being an instance of a lazy loading class that will import the elements to get their version information only at run time if needed. This will improve the overall import performance and also reduce the possibility of introducing an import cycle when we remove namespace packaging. Related to Qiskit#5100 and Qiskit#5532
mtreinish
added
the
Changelog: API Change
The qiskit.execute module has a name conflict with the re-exported path to it's sole public function qiskit.execute(). Whether python uses the module or the function is an import time race condition and by making the element imports occur at runtime this switches the normal evaluation to use the module instead of the function. However, this was always a bug and could have been triggered in other ways too. To make the imports deterministic either the function or the module needs to be renamed and since the module is less commonly used than the function that is renamed.
The qiskit.compiler module has a name conflict with the re-exported path to public functions qiskit.compiler. Whether python uses the module or the function is an import time race condition, amd could be triggered by subtle changes in import evaluation order. To make the imports deterministic either the function or the module needs to be renamed and since the module is less commonly used than the function that is renamed.
…t-core into lazy-wrap-qiskit-version
kdk
reviewed
Feb 2, 2021
releasenotes/notes/qiskit-version-wrapper-90cb7fcffeaafd6a.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/qiskit-version-wrapper-90cb7fcffeaafd6a.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/qiskit-version-wrapper-90cb7fcffeaafd6a.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/qiskit-version-wrapper-90cb7fcffeaafd6a.yaml
Outdated
Show resolved
Hide resolved
|
|
||
| def __init__(self): | ||
| self._version_dict = { | ||
| 'qiskit-terra': __version__, |
There was a problem hiding this comment.
Should the get_version_info() call that populates __version__ also be moved to runtime (or maybe this needs python 3.7)?
There was a problem hiding this comment.
Ideally yes it would be good to do this only at runtime, although for the release case (or anything outside of the dev case) this only adds an extra stat() call which is limited overhead. But I think we'll have to wait until python 3.7 with module getattr to do this.
We could create a custom class with a __str__ and __repr__ that runs get_version_info() internally but that would cause issues for people who use string methods on version (the use case I'm thinking of is qiskit.__version__.split('.') so I think it would be best to wait
Co-authored-by: Kevin Krsulich <[email protected]>
kdk
approved these changes
Feb 2, 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'.
ElePT
pushed a commit
to ElePT/qiskit-ibm-provider
that referenced
this pull request
Oct 9, 2023
…t#5620) * Add a lazy loading wrapper class for __qiskit_version__ This commit migrates the __qiskit_version__ module attribute away from using a dict constructed at import time to being an instance of a lazy loading class that will import the elements to get their version information only at run time if needed. This will improve the overall import performance and also reduce the possibility of introducing an import cycle when we remove namespace packaging. Related to Qiskit/qiskit#5100 and Qiskit/qiskit#5532 * Rename qiskit.execute module to qiskit.execute_function The qiskit.execute module has a name conflict with the re-exported path to it's sole public function qiskit.execute(). Whether python uses the module or the function is an import time race condition and by making the element imports occur at runtime this switches the normal evaluation to use the module instead of the function. However, this was always a bug and could have been triggered in other ways too. To make the imports deterministic either the function or the module needs to be renamed and since the module is less commonly used than the function that is renamed. * Rename qiskit.compiler inner modules to avoid name conflict The qiskit.compiler module has a name conflict with the re-exported path to public functions qiskit.compiler. Whether python uses the module or the function is an import time race condition, amd could be triggered by subtle changes in import evaluation order. To make the imports deterministic either the function or the module needs to be renamed and since the module is less commonly used than the function that is renamed. * Fix docs build * Apply suggestions from code review Co-authored-by: Kevin Krsulich <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This commit migrates the qiskit_version module attribute away from
using a dict constructed at import time to being an instance of a lazy
loading class that will import the elements to get their version
information only at run time if needed. This will improve the overall
import performance and also reduce the possibility of introducing an
import cycle when we remove namespace packaging.
Details and comments
Related to #5100 and #5532