This repository was archived by the owner on Jun 12, 2023. It is now read-only.
Move Ignis to its own package#456
Closed
mtreinish wants to merge 5 commits into
Closed
Conversation
Namespace packages are constant source of problems for users. The python packaging ecosystem around splitting packages across namespaces is fragile at the best of times and can often leave a you with an environment that isn't recoverable (especially when mixing install methods). There is also a performance hit whenever there is a piece of the namespace we allow external packages to extend since it requires doing a full python path search which can be slow depending on the backing I/O and the number of paths in sys.path for an environment. This commit addresses the piece from the ignis perspective by moving qiskit.ignis to it's own package and namespace 'qiskit_ignis'. This will be coupled with a change in terra that removes the arbitrary namespace hook points and hard coding the element namespace maps via a custom import loader at the root of the namespace. This has 2 advantages it removes the use of namespace packages so the fragility and performance impact are fixed since every element will be renamed to use 'qiskit_*' instead of 'qiskit.*', but it also makes it explicit where we extend the namespace. The previous method allowed any package to extend qiskit.* and qiskit.providers.* with whatever they wanted. This commit will need to be coordinated with the terra change to ensure we don't block development, because while it's not breaking for end users as a coordinated code release piecewise it's a breaking change for each element, including Ignis. Depends-On: Qiskit/qiskit#4767 Fixes Qiskit/qiskit#559
1478115 to
48336d4
Compare
Collaborator
Author
|
Setting this as on hold because other elements are having issues with this in their CI, so a revert is proposed at Qiskit/qiskit#5006 |
Collaborator
Author
|
With the creation of the qiskit-experiments repo this isn't needed anymore because that project will subsume this when it's ready and already exists in a separate namespace. |
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
Namespace packages are constant source of problems for users. The python
packaging ecosystem around splitting packages across namespaces is
fragile at the best of times and can often leave a you with an
environment that isn't recoverable (especially when mixing install
methods). There is also a performance hit whenever there is a piece of
the namespace we allow external packages to extend since it requires
doing a full python path search which can be slow depending on the
backing I/O and the number of paths in sys.path for an environment. This
commit addresses the piece from the ignis perspective by moving
qiskit.ignis to it's own package and namespace 'qiskit_ignis'.
This will be coupled with a change in terra that removes the arbitrary
namespace hook points and hard coding the element namespace maps via a
custom import loader at the root of the namespace. This has 2 advantages
it removes the use of namespace packages so the fragility and
performance impact are fixed since every element will be renamed to use
'qiskit_' instead of 'qiskit.', but it also makes it explicit where we
extend the namespace. The previous method allowed any package to extend
qiskit.* and qiskit.providers.* with whatever they wanted. This commit
will need to be coordinated with the terra change to ensure we don't
block development, because while it's not breaking for end users as a
coordinated code release piecewise it's a breaking change for each
element, including Ignis.
Details and comments
Depends-On: Qiskit/qiskit#4767
Fixes Qiskit/qiskit#559