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

Fix warning on __qiskit_version__ #10686

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

jakelishman
Copy link
Member

Summary

The warning emitted by the QiskitVersion class that backs the __qiskit_version__ variable was previously set to warn on instantiation, not usage. This caused the warning to trigger during library import, rather than at the site of usage.

Details and comments

Originally introduced in #10242.

The warning emitted by the `QiskitVersion` class that backs the
`__qiskit_version__` variable was previously set to warn on
instantiation, not usage.  This caused the warning to trigger during
library import, rather than at the site of usage.
@jakelishman jakelishman added the Changelog: None Do not include in changelog label Aug 22, 2023
@jakelishman jakelishman requested a review from a team as a code owner August 22, 2023 11:47
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@mtreinish mtreinish added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Aug 22, 2023
@mtreinish mtreinish added this to the 0.25.2 milestone Aug 22, 2023
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.

This LGTM, in my review on #10242 I was mainly concerned at the time that it didn't show up on import. But I realize I missed the stacklevel kwarg there so it was never actually visible with the default filters and was always triggering. Thanks for catching and fixing this.

@jakelishman
Copy link
Member Author

Yeah, I caught it because pytest begins catching warnings during test collection and shows them - I was trying to fix a couple of backlog bugs from the OQ3 import, which uses pytest as its running.

@mtreinish mtreinish enabled auto-merge August 22, 2023 13:10
@mtreinish mtreinish added this pull request to the merge queue Aug 22, 2023
Merged via the queue into Qiskit:main with commit 4ab2ef8 Aug 22, 2023
mergify bot pushed a commit that referenced this pull request Aug 22, 2023
* Fix warning on `__qiskit_version__`

The warning emitted by the `QiskitVersion` class that backs the
`__qiskit_version__` variable was previously set to warn on
instantiation, not usage.  This caused the warning to trigger during
library import, rather than at the site of usage.

* Add warning assertion to test

(cherry picked from commit 4ab2ef8)
github-merge-queue bot pushed a commit that referenced this pull request Aug 22, 2023
* Fix warning on `__qiskit_version__`

The warning emitted by the `QiskitVersion` class that backs the
`__qiskit_version__` variable was previously set to warn on
instantiation, not usage.  This caused the warning to trigger during
library import, rather than at the site of usage.

* Add warning assertion to test

(cherry picked from commit 4ab2ef8)

Co-authored-by: Jake Lishman <[email protected]>
@jakelishman jakelishman deleted the fix-qiskit-version-warning branch August 31, 2023 10:45
SamD-1998 pushed a commit to SamD-1998/qiskit-terra that referenced this pull request Sep 7, 2023
* Fix warning on `__qiskit_version__`

The warning emitted by the `QiskitVersion` class that backs the
`__qiskit_version__` variable was previously set to warn on
instantiation, not usage.  This caused the warning to trigger during
library import, rather than at the site of usage.

* Add warning assertion to test
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 stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants