Move metapackage shim for combined releases#10530
Conversation
Now that Qiskit 0.44.0 has been released, the Qiskit project is now what was formerly qiskit-terra. However, because Python packaging lacks a clean mechanism to enable one package superseding another we're not able to stop shipping a qiskit-terra package that owns the qiskit python namespace without introducing a lot of potential user friction. So moving forward the qiskit project will release 2 packages an inner qiskit-terra which still contains all the library code and public facing qiskit package which installs that inner package only. To enable this workflow this commit migrates the metapackage setup.py into the terra repository and setups build automation to publish a qiskit package in addition to the inner terra package on each release tag.
mtreinish
added
the
Changelog: None
|
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 5824410178
💛 - Coveralls |
kevinhartman
left a comment
kevinhartman
left a comment
There was a problem hiding this comment.
This LGTM overall. Makes sense to remove the toqm extra since we don't have a code dependency on it anymore thanks to the staged pass manager plugin interface 😄.
| This library is the core component of Qiskit, **Terra**, which contains the building blocks for creating | ||
| and working with quantum circuits, programs, and algorithms. It also contains a compiler that supports | ||
| different quantum computers and a common interface for running programs on different quantum computer architectures. | ||
| This Qiskit contains the building blocks for creating and working with quantum circuits, programs, and algorithms. It also |
There was a problem hiding this comment.
| This Qiskit contains the building blocks for creating and working with quantum circuits, programs, and algorithms. It also | |
| Qiskit contains the building blocks for creating and working with quantum circuits, programs, and algorithms. It also |
| name="qiskit", | ||
| version="0.45.0", | ||
| description="Software for developing quantum computing programs", | ||
| long_description=README, |
There was a problem hiding this comment.
So this will be the string "../README.md", right? Does setup take either a file path or the contents directly and decide what to do from there? Maybe I'm missing something 😄
There was a problem hiding this comment.
Those files are actually symlinks in the repo, that's just how GitHub displays them. No idea how Windows will handle them, tbh.
There was a problem hiding this comment.
It apparently works on newer versions of windows: https://stackoverflow.com/questions/5917249/git-symbolic-links-in-windows/59761201#59761201
There was a problem hiding this comment.
Did you consider maybe switching the way we publish the packages, so qiskit-terra becomes the dummy package with a simple dependency on qiskit, and what's currently Terra becomes qiskit? I wrote a little more in a comment below, but I think it works right. It feels like the kind of thing where I could easily have not thought of something, though.
| name="qiskit", | ||
| version="0.45.0", | ||
| description="Software for developing quantum computing programs", | ||
| long_description=README, |
There was a problem hiding this comment.
Those files are actually symlinks in the repo, that's just how GitHub displays them. No idea how Windows will handle them, tbh.
| requirements = ["qiskit-terra==0.45.0"] | ||
|
|
||
| setup( | ||
| name="qiskit", |
There was a problem hiding this comment.
Would it be possible / better to publish our wheels as qiskit, flip the dependency order, and have qiskit-terra become the thin dependency-only package? In other words, swap the name of what's currently qiskit-terra to become qiskit, and have this "new" package be qiskit-terra which just contains the dependency on qiskit?
As far as I could think, it'll satisfy all the upgrade paths correctly, and it also has the pleasant properties:
- the preferred install,
pip install qiskit, installs directly (and the name Terra doesn't appear any more) - our preferred install path links directly to the wheels, so going to the PyPI page is less surprising
- similarly, the PyPI page for qiskit-terra will much more clearly show that it's been superseded (and we could even change the README there to simply say that, rather than the symlink trick we're using here because we're still treating
qiskit-terraas the main package) - we can drop
qiskit-terrain the future, say at 1.0 or 2.0, andpip install qiskitwill still work correctly - we effectively gain another ~10GB storage on PyPI, since we've barely used any of our space on the metapackage
- it matches the directory structure of this repo more naturally
There was a problem hiding this comment.
It's the lack of uninstall of the old qiskit-terra package that's potentially the problem, right?
I wonder if we might actually just be better doing it anyway and saying "you might want to manually uninstall qiskit-terra when upgrading"?
There was a problem hiding this comment.
Yeah I considered it, but the upgrade path is particularly fraught here which is why I pushed forward in this direction. It's actually not just the lack of an uninstall but also the potential uninstall. The obvious issues is a user who does pip install -U qiskit from 0.44.0 -> 0.45.0 in this case if we have a reverse dependency then the lack of an uninstall for qiskit-terra is a potential problem because pip will just dump the qiskit package's contents into site-libs and potentially leaving mismatched versions. But the other side is if you do an uninstall of qiskit-terra <= 0.25.0 (which can happen automatically as part of qiskit-terra package upgrade) after moving code to the qiskit package you'll potentially have a big chunk of the terra code deleted. There are also a lot of other edge cases that can come up to depending on the install method (as there are multiple install mechanisms) which are similarly fraught. Also not to mention the mix of editable installs with regular installs between the packages (ie doing an editable install of qiskit-terra==0.25.0 and a regular install of qiskit==0.45.0) which is likely an issue independently.
The reason I went with this path is it's the only way to maintain an easy upgrade path that doesn't require any manual updates. The only way to ensure users have a good upgrade path if we wanted to reverse the dependency is to tell them to manually uninstall qiskit-terra in all cases before updating. I'm opposed to reversing the dependency for 2 reasons, first because for the user of qiskit they never manually installed qiskit-terra and may not realize it's installed and haven't been managing it at all. The second is we actually did this in the past for qiskit 0.7.0 the code moved from the qiskit package to the qiskit-terra package and there were a lot of user issues around the migration. There was actually a workaround at the time to have the qiskit's setup.py fail wheel builds to avoid an automatic uninstall of qiskit after having upgraded qiskit-terra (which is the reason we don't published wheels for the metapackage, although this PR fixes that oversight). But, having had to deal with the headaches around doing this exact thing in the past I don't think it's something we want to subject users to again, when if we keep the dependency relationship the same as today everything works, it's just a bit more awkward looking.
There was a problem hiding this comment.
I really wish setuptools had a supersedes option that let us say qiskit supersedes the qiskit-terra package so it would correctly handle the upgrade path. I guess it's enough of an edge case that it doesn't come up very often, but for real package managers they typically offer this in the metadata because it does happen.
There was a problem hiding this comment.
That's all fair. I do think it's something we should consider for Qiskit 1.0, though - I think we have a chance there to say "to upgrade an existing environment to Qiskit 1.0, you first must pip uninstall qiskit qiskit-terra qiskit-aer qiskit-ibmq-provider" and get away with it more from a user perspective. I also think it'll generally be a bit easier than last time, because we're not fighting the namespace packaging any more. It's pain now, but it frees us a lot more for the future, and the final packaging story which be much more straightforward.
I'm not super worried about editable installs breaking, because that's just dev workflows, and they've already been broken for certain configurations for several months now as we removed the namespace packaging (having a regular package installed that used the namespace packaging broke the editable install).
Also yeah, it would be good if there was a proper way to have one package supersede another.
There was a problem hiding this comment.
Here are my thoughts (as someone who will not hear anywhere near as many user complaints as either of you 🙂):
I don't love the dependency reversal because of the scenario @mtreinish mentioned of pip install -U qiskit overwriting an old qiskit-terra's contents without removing it and also because it will break all the downstream projects that depend now explicitly on (got confused about this and forgot about in this scenario the dependency was reversed so this wouldn't happen). What about this progression:qiskit-terra without an upper version bound
- Add a check to
qiskit/__init__.pythat warns ifqiskitis not installed but leave all the code inqiskit-terraand keepqiskitas a package depending only on a specific version ofqiskit-terra. - After enough time for the community to have switched over to depending on and install
qiskitrather thanqiskit-terra(a year?), make a release with the code moved into theqiskitpackage withqiskit-terrastill as a dependency but now empty. - After enough time again (another year?) that most users should have a release with the code in the
qiskitpackage, drop the terra dependency.
The goal is to make sure that everyone is using qiskit before shifting the code over, so that the upgrade to an empty terra does not leave someone with no files. It requires a lot of patience though.
Personally, I think it would be better to get rid of terra if not too painful because it will be a point of minor confusion, mostly as a distraction but also some potential confusion since pip does allow some ways of installing packages without dependencies which could lead a user to end up with qiskit and qiskit-terra with different version numbers.
There was a problem hiding this comment.
Will: I pretty much agree with everything you said, the reason I was pushing for a more compressed timeline was that I think we'll get rather more acceptance from users between the 0.x -> 1.0 transition that they might need to pip uninstall x y z; pip install x to upgrade than even the 1.0 -> 2.0 transition, even if there are warnings. I'm also in favour of getting rid of qiskit-terra, because I think it'll hopefully be a minor confusion in most cases, but in cases where it does matter, we're going to have to talk users through more pain that it'll cause here, and it'll be a permanent albatross for us if we don't cut it when we deem we "have the chance".
That said, for this immediate PR, we're going to punt the decision a little bit, since the current form is what the metapackage currently does, and we wouldn't do this until at least the 1.0 release, if we do it at all.
* some follow up on Qiskit#10530 * extend some badges * This Qiskit contains the building blocks for creating and working with quantum circuits, programs, and algorithms. -> This framework allows for building, transforming, and visualizing quantum circuits. * The explanation of the Bell state is moving to IBM Quantun learning platform * I think examples/ should eventually be replaced by proper tutorials * Add StackOverflow as a forum * Remove link to https://github.com/Qiskit/qiskit-tutorials * Update README.md Co-authored-by: Matthew Treinish <[email protected]> * Update README.md * Update README.md * broken lines in badges * doi --------- Co-authored-by: Matthew Treinish <[email protected]>
jakelishman
left a comment
jakelishman
left a comment
There was a problem hiding this comment.
Some minor tweaks that I can spot right now, though I imagine we'll probably need to bugfix live during the first attempt at this deployment, because it's always hard to spot everything.
What's the plan between this PR and #10584, since we need this PR more urgently but there's a major logical conflict between the two?
| --- | ||
| upgrade: | ||
| - | | ||
| The ``toqm`` optional setuptools extra that previously enabled running | ||
| ``pip install qiskit-terra[topm]`` has been removed as nothing in the | ||
| Qiskit code base is currently using that package anymore. If you'd like | ||
| to use the qiskit TOQM transpiler plugin you should install the | ||
| ``qiskit-toqm`` package directly. |
There was a problem hiding this comment.
the bip-mapper extra is also gone - was that intentional, and does it need a reno if so?
There was a problem hiding this comment.
I did the release note in: #10526 which is the larger removal of the pass.
There was a problem hiding this comment.
Ok, we'll need to remember to re-add it when we backport this to do the 0.25.1 deployment.
jakelishman
left a comment
jakelishman
left a comment
There was a problem hiding this comment.
A minor thing about a dodgy comment, but other than that and the query about the ordering of this PR and the other README one, I'm happy with this.
mtreinish
added
the
stable backport potential
* Move metapackage shim for combined releases Now that Qiskit 0.44.0 has been released, the Qiskit project is now what was formerly qiskit-terra. However, because Python packaging lacks a clean mechanism to enable one package superseding another we're not able to stop shipping a qiskit-terra package that owns the qiskit python namespace without introducing a lot of potential user friction. So moving forward the qiskit project will release 2 packages an inner qiskit-terra which still contains all the library code and public facing qiskit package which installs that inner package only. To enable this workflow this commit migrates the metapackage setup.py into the terra repository and setups build automation to publish a qiskit package in addition to the inner terra package on each release tag. * some follow up on #10530 (#19) * some follow up on #10530 * extend some badges * This Qiskit contains the building blocks for creating and working with quantum circuits, programs, and algorithms. -> This framework allows for building, transforming, and visualizing quantum circuits. * The explanation of the Bell state is moving to IBM Quantun learning platform * I think examples/ should eventually be replaced by proper tutorials * Add StackOverflow as a forum * Remove link to https://github.com/Qiskit/qiskit-tutorials * Update README.md Co-authored-by: Matthew Treinish <[email protected]> * Update README.md * Update README.md * broken lines in badges * doi --------- Co-authored-by: Matthew Treinish <[email protected]> * Expand lint checks to entire qiskit_pkg dir * Unify extras in terra setup.py * Update CI lint job * Remove unused json imports * Cleanup manifest file * Finish comment --------- Co-authored-by: Luciano Bello <[email protected]> (cherry picked from commit 8a180ee) # Conflicts: # README.md
* Move metapackage shim for combined releases (#10530) * Move metapackage shim for combined releases Now that Qiskit 0.44.0 has been released, the Qiskit project is now what was formerly qiskit-terra. However, because Python packaging lacks a clean mechanism to enable one package superseding another we're not able to stop shipping a qiskit-terra package that owns the qiskit python namespace without introducing a lot of potential user friction. So moving forward the qiskit project will release 2 packages an inner qiskit-terra which still contains all the library code and public facing qiskit package which installs that inner package only. To enable this workflow this commit migrates the metapackage setup.py into the terra repository and setups build automation to publish a qiskit package in addition to the inner terra package on each release tag. * some follow up on #10530 (#19) * some follow up on #10530 * extend some badges * This Qiskit contains the building blocks for creating and working with quantum circuits, programs, and algorithms. -> This framework allows for building, transforming, and visualizing quantum circuits. * The explanation of the Bell state is moving to IBM Quantun learning platform * I think examples/ should eventually be replaced by proper tutorials * Add StackOverflow as a forum * Remove link to https://github.com/Qiskit/qiskit-tutorials * Update README.md Co-authored-by: Matthew Treinish <[email protected]> * Update README.md * Update README.md * broken lines in badges * doi --------- Co-authored-by: Matthew Treinish <[email protected]> * Expand lint checks to entire qiskit_pkg dir * Unify extras in terra setup.py * Update CI lint job * Remove unused json imports * Cleanup manifest file * Finish comment --------- Co-authored-by: Luciano Bello <[email protected]> (cherry picked from commit 8a180ee) # Conflicts: # README.md * Fix merge conflicts --------- Co-authored-by: Matthew Treinish <[email protected]>
* Move metapackage shim for combined releases Now that Qiskit 0.44.0 has been released, the Qiskit project is now what was formerly qiskit-terra. However, because Python packaging lacks a clean mechanism to enable one package superseding another we're not able to stop shipping a qiskit-terra package that owns the qiskit python namespace without introducing a lot of potential user friction. So moving forward the qiskit project will release 2 packages an inner qiskit-terra which still contains all the library code and public facing qiskit package which installs that inner package only. To enable this workflow this commit migrates the metapackage setup.py into the terra repository and setups build automation to publish a qiskit package in addition to the inner terra package on each release tag. * some follow up on Qiskit#10530 (Qiskit#19) * some follow up on Qiskit#10530 * extend some badges * This Qiskit contains the building blocks for creating and working with quantum circuits, programs, and algorithms. -> This framework allows for building, transforming, and visualizing quantum circuits. * The explanation of the Bell state is moving to IBM Quantun learning platform * I think examples/ should eventually be replaced by proper tutorials * Add StackOverflow as a forum * Remove link to https://github.com/Qiskit/qiskit-tutorials * Update README.md Co-authored-by: Matthew Treinish <[email protected]> * Update README.md * Update README.md * broken lines in badges * doi --------- Co-authored-by: Matthew Treinish <[email protected]> * Expand lint checks to entire qiskit_pkg dir * Unify extras in terra setup.py * Update CI lint job * Remove unused json imports * Cleanup manifest file * Finish comment --------- Co-authored-by: Luciano Bello <[email protected]>
7 participants
Summary
Now that Qiskit 0.44.0 has been released, the Qiskit project is now what was formerly qiskit-terra. However, because Python packaging lacks a clean mechanism to enable one package superseding another we're not able to stop shipping a qiskit-terra package that owns the qiskit python namespace without introducing a lot of potential user friction. So moving forward the qiskit project will release 2 packages an inner qiskit-terra which still contains all the library code and public facing qiskit package which installs that inner package only. To enable this workflow this commit migrates the metapackage setup.py into the terra repository and setups build automation to publish a qiskit package in addition to the inner terra package on each release tag.
Details and comments
Close Qiskit/qiskit-metapackage#1792.