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

Remove qiskit-toqm tests and requirements-dev.txt entry (backport #9057) #9061

Merged
merged 4 commits into from
Nov 3, 2022

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Nov 2, 2022

This is an automatic backport of pull request #9057 done by Mergify.
Cherry-pick of 8999047 has failed:

On branch mergify/bp/stable/0.22/pr-9057
Your branch is up to date with 'origin/stable/0.22'.

You are currently cherry-picking commit 8999047a2.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   test/python/transpiler/test_preset_passmanagers.py

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   requirements-dev.txt

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally


Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

* Remove qiskit-toqm tests and requirements-dev.txt entry

Im #9042 we removed the special case code to enable using toqm as an
optional routing method. This was needed prior to qiskit-terra 0.22.0 as
we didn't have the transpiler stage plugin interface. However, now that
we've added a dedicated interface for external transpiler passes to
integrate into transpile() the toqm passes are using that interface.
However, in #9042 the tests for verifying the previously hard-coded toqm
routing method path worked as before were left intact. In general this
would be a good approach to ensure we're maintaining backwards
compatibilty with the new modular interface with earlier releases.
However, in this specific case this is causing an issue with how tests
are run. Since qiskit-toqm has a necessary dependency on qiskit-terra
this means when we install our development requirements before running
tests toqm is pulling in the latest stable release of qiskit-terra from
pypi. Then we later upgrade that with the source build before running
tests. This however this bi-directional test dependency introduces a
tension in a number of places. For the most recent example, when we're
trying to add support for new platforms (see #9028 for an example)
where the stable version of qiskit-terra does not support the platform.
We're unable to run CI with qiskit-toqm being installed first since
installing stable qiskit-terra won't work/

This commit removes qiskit-toqm from the requirements-dev.txt list and
also removes the test cases using it to fix this conflict. In general
the testing for qiskit-toqm can now be self contained since it's
exercising a stable interface in terra that's tested independently. When
weighing the backwards compatibility coverage vs the CI and build
complexities having the bidirectional test dependency has just removing
the toqm tests and development requirement is the simplest path forward.

* Remove unused imports

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 8999047)

# Conflicts:
#	requirements-dev.txt
@mergify mergify bot requested a review from a team as a code owner November 2, 2022 19:01
@mergify mergify bot added the conflicts used by mergify when there are conflicts in a port label Nov 2, 2022
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

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

  • @Qiskit/terra-core

This fixes a merge conflict and removes the qiskit-toqm entry from the requirements-dev.txt file
@mtreinish mtreinish added automerge Changelog: None Do not include in changelog and removed conflicts used by mergify when there are conflicts in a port labels Nov 2, 2022
mtreinish
mtreinish previously approved these changes Nov 2, 2022
@jakelishman jakelishman added this to the 0.22.2 milestone Nov 2, 2022
@jakelishman
Copy link
Member

I suppose this didn't really need to be backported if it's causing problems.

@mtreinish
Copy link
Member

mtreinish commented Nov 2, 2022

Yeah, on second thought we probably don't need this on 0.22.x. We only needed this on main to get Python 3.11 support working without tweedledum. As the bidriectional dependency from qiskit-toqm was causing tweedledum to be installed even though nothing actually was requiring it on main.

But it raises a larger issue for us when backporting #9028 in that we still require tweedledum on the stable branch. We're going to have to workaround this in the backport PR probably by adding an environment marker on python_version in the requirements list for tweedledum. However, we might still need this to get Python 3.11 working in CI on this branch.

I've removed the automerge tag for now though, we can wait until #9028 is backported to see whether we need this backport or not.

@mtreinish
Copy link
Member

In my local testing with #9071 this is still needed because of the same issue we hit in #9028 with toqm installing qiskit-terra 0.22.1 first. We can revert this post 0.22.2 if wanted. Or I can update all the CI jobs to manually install toqm after we install terra and remove it from the requirements-dev.txt list. But for right now I think we should just remove this.

@mergify mergify bot merged commit 0a731ca into stable/0.22 Nov 3, 2022
@mtreinish mtreinish deleted the mergify/bp/stable/0.22/pr-9057 branch November 23, 2022 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants