Skip to content
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

Replace qiskit metapackage with qiskit-terra #11271

Merged
merged 5 commits into from
Nov 27, 2023

Conversation

jakelishman
Copy link
Member

Summary

This commit completely removes the concept of the qiskit-terra package from Qiskit main. The hitherto metapackage qiskit is promoted to be the concrete code package.

This is a completely breaking change for packaging purposes for users, as there is no clean upgrade path from qiskit-terra to qiskit; PyPI and pip do not give us the tools to mark that the former obsoletes and supersedes the latter. We intend to follow this PR with a technical blog post somewhere explaining the changes, how users should adapt ("to install Qiskit 1.0, you must start a new venv"), and why we needed to do this.

The "why", in part, is:

  • the metapackage legacy causes awkward upgrade paths on every release; some packages still depend on qiskit-terra, some on qiskit, and pip will happily make those two get out-of-sync when upgrading a transitive dependency with only a warning that users are used to ignoring.

  • with the 1.0 release, we (believe we) will have some leeway from users to make such a breaking change.

  • having the metapackage split makes it difficult for downstream projects and developers to test against main - they always must install both qiskit-terra and qiskit, and the latter has no meaning in editable installs, so needs re-installing after each version bump. Problems surrounding this have already broken CI for Qiskit, for at least one internal IBM repo, and at least my dev install of Qiskit. This could be improved a bit with more education, but it's still always going to increase the barrier to entry and make it much harder to do the right thing.

We will not publish any 1.0 or above version of qiskit-terra. All dependents on Qiskit should switch their requirements to qiskit.

Details and comments

We will be accompanying this change with a blog post with more information, and issuing warnings from Qiskit 0.46 about the upcoming change as well.

@jakelishman jakelishman added type: qa Issues and PRs that relate to testing and code quality Changelog: API Change Include in the "Changed" section of the changelog labels Nov 17, 2023
@jakelishman jakelishman added this to the 1.0.0pre1 milestone Nov 17, 2023
@jakelishman jakelishman requested a review from a team as a code owner November 17, 2023 21:28
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

@jakelishman jakelishman changed the title Replace qiskit metapacakge with qiskit-terra Replace qiskit metapackage with qiskit-terra Nov 17, 2023
@coveralls
Copy link

coveralls commented Nov 17, 2023

Pull Request Test Coverage Report for Build 7009157389

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 21 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.02%) to 87.068%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 3 92.68%
crates/qasm2/src/parse.rs 18 96.67%
Totals Coverage Status
Change from base Build 7004497279: -0.02%
Covered Lines: 60801
Relevant Lines: 69832

💛 - Coveralls

@jakelishman
Copy link
Member Author

jakelishman commented Nov 17, 2023

Ah, this cannot succeed until we can use Aer 0.13, because Aer 0.12.2 still has an old dependency on qiskit-terra, which means we're on hold pending Qiskit/qiskit-aer#1983 for now.

@jakelishman jakelishman added the on hold Can not fix yet label Nov 17, 2023
@jakelishman jakelishman removed the on hold Can not fix yet label Nov 24, 2023
This commit completely removes the concept of the `qiskit-terra` package
from Qiskit main.  The hitherto metapackage `qiskit` is promoted to be
the concrete code package.

This is a completely breaking change for packaging purposes for users,
as there is no clean upgrade path from `qiskit-terra` to `qiskit`; PyPI
and pip do not give us the tools to mark that the former obsoletes and
supersedes the latter.  We intend to follow this PR with a technical
blog post somewhere explaining the changes, how users should adapt
("to install Qiskit 1.0, you must start a new venv"), and why we needed
to do this.

The "why", in part, is:

- the metapackage legacy causes awkward upgrade paths on every release;
  some packages still depend on `qiskit-terra`, some on `qiskit`, and
  pip will happily make those two get out-of-sync when upgrading a
  transitive dependency with only a warning that users are used to
  ignoring.

- with the 1.0 release, we (believe we) will have some leeway from users
  to make such a breaking change.

- having the metapackage split makes it difficult for downstream
  projects and developers to test against `main` - they always must
  install both `qiskit-terra` and `qiskit`, and the latter has no
  meaning in editable installs, so needs re-installing after each
  version bump.  Problems surrounding this have already broken CI for
  Qiskit, for at least one internal IBM repo, and at least my dev
  install of Qiskit.  This could be improved a bit with more education,
  but it's still always going to increase the barrier to entry and make
  it much harder to do the right thing.

We will not publish any 1.0 or above version of `qiskit-terra`.  All
dependents on Qiskit should switch their requirements to `qiskit`.
@jakelishman
Copy link
Member Author

Relevant fix to Neko is Qiskit/qiskit-neko#36.

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this LGTM, one tiny nit in the docs about the windows usage of venv,

# Install Qiskit (only once).
pip install 'qiskit>=1.0'

On Windows, replace ``source <venv>/bin/activate`` with ``source <venv>/Scripts/activate``.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
On Windows, replace ``source <venv>/bin/activate`` with ``source <venv>/Scripts/activate``.
On Windows, replace ``source <venv>/bin/activate`` with ``source <venv>\Scripts\activate``.

Although I guess it really depends on which windows shell you're using it'd be <venv>\Scripts\activate.bat if you're using cmd.exe or PS <venv>\Scripts\Activate.ps1 if you're using powershell. This would work if you're using bash on windows I guess though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true, I forgot people used things other than bash-likes. Shall we do something more here? I'm not sure what to write without going too overboard - I don't really know how most Windows users interact with venv.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I'm debugging something on windows personally I either do it via git bash (so what you had would work) or via cmd.exe and call the activate.bat file. It often depends on what I'm debugging as either shell might be needed to reproduce an issue if it involves output or default character encoding.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it might be better to refer to the stdlib python docs here https://docs.python.org/3/library/venv.html#how-venvs-work

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea - done in 6f5b066.

jakelishman and others added 2 commits November 27, 2023 18:05
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the quick update.

auto-merge was automatically disabled November 27, 2023 20:17

Pull Request is not mergeable

@jakelishman jakelishman added this pull request to the merge queue Nov 27, 2023
Merged via the queue into Qiskit:main with commit e91947e Nov 27, 2023
14 checks passed
@jakelishman jakelishman deleted the terra-nullius branch November 27, 2023 22:22
garrison added a commit to Qiskit/qiskit-addon-cutting that referenced this pull request Nov 28, 2023
@jakelishman jakelishman mentioned this pull request Nov 28, 2023
1 task
github-merge-queue bot pushed a commit to qiskit-community/qiskit-experiments that referenced this pull request Nov 28, 2023
### Summary

Now that the metapackage has been removed in
Qiskit/qiskit#11271, we should be able to run
the daily cron jobs against `qiskit` and not `qiskit-terra`.
mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Dec 7, 2023
In Qiskit#11119 we removed the use_develop flag in the tox config which was
used to tell tox to use a single editable install for running a job
(avoiding the need to build from sdist on every run) as part of a
temporary refactoring to ensure we installed the metapackage along with
terra. Now that we've standardized the packaging in Qiskit#11271 that was
undone, but the usedevelop flag was not added back. This has caused tox
to rebuild the package from sdist on every run even if there are no code
changes. This commit restores the missing option to fix this.
itoko pushed a commit to itoko/qiskit-experiments that referenced this pull request Dec 12, 2023
…y#1329)

### Summary

Now that the metapackage has been removed in
Qiskit/qiskit#11271, we should be able to run
the daily cron jobs against `qiskit` and not `qiskit-terra`.
nkanazawa1989 pushed a commit to nkanazawa1989/qiskit-experiments that referenced this pull request Jan 17, 2024
…y#1329)

### Summary

Now that the metapackage has been removed in
Qiskit/qiskit#11271, we should be able to run
the daily cron jobs against `qiskit` and not `qiskit-terra`.
caleb-johnson added a commit to Qiskit/qiskit-addon-cutting that referenced this pull request Feb 12, 2024
* Unwrap the `Tensor` objects from Qiskit Nature as necessary

Fixes #406

* Revert "Temporarily remove Qiskit Nature from dev version tests (#407)"

This reverts commit 88e2195.

* `unfold` the `S8Integrals`

* Install qiskit only, not qiskit-terra

See Qiskit/qiskit#11271.

* Unpin qiskit-nature

We'll probably also need to bump the version to 0.7 or higher,
but let's do that once the code is ready.

* Update forging to support Qiskit 1.0 and Nature 0.7

* Remove serverless tutorials

* Update ruff config

* Ignore forging tutorial 1

* Update qiskit readme shield. Update dev version of Qiskit to point to main branch

* Move ruff changes out of this PR

* Remove 0.46 from dev version testing

* Update path to qiskit TwoQubitWeylDecomposition

* Use 0.46 targets until 1.0 is released.

* Get all tests passing by removing problematic test modules

* ruff

* Bump rustwork version to be compatible w Qiskit 1.0

* Bump dev version to 1.0 from 0.46

* Bug in dev workflow

* Remove do_while from PassManager used to remove resets

* Fix notebooks

* Put do_while back

* Fix recursion error with forging

* Dont ignore forging tutorial

* Install Nature from pypi

* Make compatible with 0.46+1.0

* .github/workflows/test_development_versions.yml

* Bug in cutting_experiments

* Use DoWhileController

* Fix impmort of DWController

* Bump min qiskit version

* bump max vers

---------

Co-authored-by: Jim Garrison <garrison@ibm.com>
Co-authored-by: Jim Garrison <jim@garrison.cc>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: API Change Include in the "Changed" section of the changelog priority: high type: qa Issues and PRs that relate to testing and code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants