-
Notifications
You must be signed in to change notification settings - Fork 127
Add migration guide on Qiskit packaging #710
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
Conversation
This is a pretty large guide on migration recommendations and troubleshooting around the Qiskit 1.0 release.
The spell-checker is being racist against me 🇬🇧 🇬🇧 🇬🇧 haha (I'll fix that later.) |
``` | ||
|
||
4. If you are not planning to use the environment immediately, use the `deactivate` command to leave it. | ||
|
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.
The only potential gotcha here for users that I think is good to call out here is compatibility with other dependencies that depend on qiskit-terra and/or qiskit<1
. I think this will trip up users the most more than not being able to pip install -U qiskit
because it's the piece that requires understanding how pip does dep solving or lack thereof and people doing things like python -m venv test && test/bin/pip install qiskit>=1.0 && test/bin/pip install qiskit-qubit-reuse
(this is a good reminder I should update the plugin package to not depend on terra before 1.0). This will end up in a mixed installation state between qiskit 1.0.0 and qiskit-terra 0.46.0. I think this really should be called out in the top and not just in the troubleshooting guide below.
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.
I've thrown in an admonition warning about this, including a link to the troubleshooting section if people do get into problems in 75f6e3a. Do you think it's enough?
I don't necessarily want to get too pessimistic up at the top of the guide - if they're just using IBM tooling (which I imagine most users are), they're hopefully not going to have any issues by the time 1.0 is actually released.
This section uses some Python-packaging jargon to better explain what was happening. | ||
The following words have special meanings: | ||
|
||
* *module*: A single Python file. | ||
|
||
* *package*: A directory containing an `__init__.py` and other files or packages that Python can read. | ||
This is the actual code as installed on your system, and what executes when you run `import something`. | ||
Python considers any directory that is on the *search path* to be something you can import (and will import quite a few more things besides!). | ||
|
||
This is not the same object that you `pip install` (which is a *distribution*), but commonly the thing you `pip install` and the thing you `import` will have the same name. | ||
|
||
* *submodule*, *subpackage*: These are somewhat imprecise terms, but are commonly used. | ||
The *sub* part just means "contained inside a package". | ||
A *submodule* is just a module and a *subpackage* is just a package, but they are part of a larger *package*. |
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.
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.
I can reformat this into a table if that would seem more appropriate? terms in the left column, defs in the right?
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.
I'm happy with whatever styling you think would be best. My guess is that a table might look a bit unbalanced and hard to read by squeezing the "definition" paragraphs a bit much, but I don't know and I'm fine deferring to you guys in design anyway.
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.
I don't have any suggestions on the For developers section—it looks good.
Qiskit 1.0 includes breaking changes, and several packages will have marked themselves as not-yet-compatible with it, so you might see errors from `pip` when running this until they have new versions. | ||
Old versions of packages may also depend on the legacy `qiskit-terra` package, which might not immediately error out during this command, but might raise an error when running `import qiskit`. | ||
|
||
If you know of any packages that are not yet compatible with Qiskit 1.0, you will have to not install those yet, and wait until the packages are updated. |
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.
Perhaps here you can link to the section on how to use pipdeptree
to find dependency versions.
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.
I put in an extra comment and link in f4b16f0, though I avoided dropping an actual link to the specific troubleshooting section below, because that's all about recovering from a broken environment. I don't actually know a simple way to get pip
(or pipdeptree
) to tell you the requirements of a particular prospective package without attempting to install it (though maybe the experimental pip index
will eventually do that).
@javabster requested that this be split into two documents: One for devs and one for users. Would you like me to take a few minutes to do that? |
Imo, the scope covered is logically grouped into either one (as here) or four (splitting each of the second-level headings onto its own page). In case it's not clear from the unrendered MDX, the page structure currently looks like:
My concern with splitting it in two ("for users" and "for developers") is that the other two sections don't fold neatly into either of those categories; they're both aimed at everyone. They also don't have a natural grouping that would put only them together, and not with the two "for x" sections. If the concern is primarily the length: "why this happened?" is not meant to be required reading - it's like an extension - and "troubleshooting" is meant to be something you find by searching for error messages, not something read end-to-end. The page intro is meant to help with conveying that. Personally I think the one-page form is the best for discoverability for readers, and grouping on the scale of "Qiskit Runtime / Qiskit 0.44 / Qiskit 1.0 / Qiskit 2.0" (the migration-guides sidebar of next year) given that there'll be at least one other Qiskit 1.0 migration guide to help with API changes, but I'm not going to oppose a final decision from your team - you guys are in charge of the documentation architecture and how it's presented. |
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.
Very thorough! I don't know how many users will read everything here to start out but it will be nice to point to when they ask for help.
The Qiskit 1.0 side of the test also checks for sentinel files that were present in old Qiskit versions and not Qiskit 1.0. | ||
|
||
If you are a Qiskit developer, it's possible that you have old `qiskit.egg-info` or `qiskit-terra.egg-info` (or `*.dist-info`) directories present on your meta path (see `sys.meta_path`), left over from old editable installations. | ||
In particular, check your working directory for any `*.egg-info` and `*.dist-info` directories. |
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.
I don't know if anyone is reading this far down in the guide, but I think the problem could also be __pycache__
directories if you use pip install -e
. You could suggest git clean -fxd
after the user checks that there are no important uncommitted files.
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.
__pycache__
shouldn't cause problems here, because the first importlib
search is for distributions not packages, and then the second is a search for "is qiskit.tools
a loadable module with a backing source file?". It's not enough for the qiskit/tools
directory to be on the path, even if qiskit/tools/__pycache__/__init__.*.pyc
exists, because we test that the ModuleSpec
has a proper location for the file (qiskit/tools.py
or qiskit/tools/__init__.py
) to verify it's not an empty namespace package left over from a dirty source tree.
I'm hesitant to suggest git clean
for general use because it's too easy to accidentally take out local changes, or clean out all your cached tox environments, etc.
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.
My comment was misleading. When I said "this problem," I was thinking of the problem of left over files in a repo that had been installed in editable mode rather than the specific import qiskit
problem with left over namespace package metadata.
I didn't fully test it but somehow there were some code removals that didn't trigger import errors for me with my editable install, and I had assumed the imports were still working because that is how things worked with Python 2 and the .pyc files left over in directories that had had the .py files removed. Maybe it was actually the info directories or something else. I didn't try to recreate the problem after doing the clean.
That particular case was coming up for me with other packages that set up Qiskit plugins in the package metadata (specifically qiskit-ibm-provider's transpiler pass plugin that imported from qiskit.extensions
). It kept working in my editable install but then failed in the clean CI environment. It is a surprising case because I was not using the plugin or anything else from qiskit-ibm-provider but just having the distribution with the plugin installed is enough to trigger an error when trying to do a transpile. I was wondering if there was anything to mention related to that in the troubleshooting section, but I think ultimately it is a consequence of having a package that is not compatible with Qiskit 1.0 installed, so that is already covered.
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.
Could you send me a set of repro steps, and I'll look into either making the import check smarter, or adding an extra bit to the troubleshooting section in this guide in a day or so, after an initial version is public?
|
||
|
||
<span id="why-did-this-happen"></span> | ||
## Why these changes happened |
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.
I like all the detail here. I wonder if there should be a section (maybe linkable) that just says that the files that were in qiskit-terra are now in qiskit and that pip does not know how to handle overlapping files and will overwrite and erase files from qiskit when operating on qiskit-terra and vice versa, as the quick version of why the careful steps described in the user section are necessary and then those interested can expand out from there.
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.
I think that's an explanation of what about the environment is actually broken, more than "why we did this", maybe? Do you think we could leave that out, or at least add it a bit later? (I'm keen to get this merged and published.)
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.
Right, I just suggested it here because I had originally suggested it up above before getting to this section and seeing that you had largely covered all the details of what was going on with packaging here. I'll leave it up to you. I don't know what is best to tell users. Mainly what I was thinking is that if a user understood about the same file paths being in both packages and pip not trying to avoid conflicts it might give intuition for understanding problems that arise.
Abby and Becky: I've updated this with an extra couple of sentences from some of Will's suggestions above, but for the other points he's raised, I think it would be best to get this published and potentially expand the troubleshooting / explanation after we've got a first version published. We need a live link for Qiskit ASAP so we can do a couple of releases. |
For the purposes of releasing quickly, then, I'll approve this now. |
I've set up the shortlink https://qisk.it/packaging-1-0 to point to where this page should appear (assuming the filename doesn't change, but I can just update the shortlink redirection if it does), so from the Qiskit side, I just need the page to appear now haha. |
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.
Good to go for the first release!
Thanks! That's great - at our absolute fastest, the relevant Qiskit release would still be a couple of hours away, but more probably it'll be tomorrow. |
Would you like me to merge it? Then I can start the process of syncing so that it publishes. Or feel free to merge! |
sure, thanks! |
Part of Qiskit#704. This is a pretty large guide on migration recommendations and troubleshooting around the Qiskit 1.0 release. I'll ask the other Qiskit developers and a few others to have a look at this for technical content as well. Given the length of this, I suspect it might be better as a stand-alone migration guide. --------- Co-authored-by: Rebecca Dimock <beckyd@us.ibm.com> Co-authored-by: ABBY CROSS <across@us.ibm.com>
Part of #704.
This is a pretty large guide on migration recommendations and troubleshooting around the Qiskit 1.0 release.
I'll ask the other Qiskit developers and a few others to have a look at this for technical content as well.
Given the length of this, I suspect it might be better as a stand-alone migration guide.