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 tweedledum runtime detection in BooleanExpression.from_dimacs_file #10132

Merged
merged 1 commit into from
May 22, 2023

Conversation

mtreinish
Copy link
Member

Summary

This commit fixes an oversight in the 0.24.0 release that caused an accidental change in the exception raised when attempting to use BooleanExpression.from_dimacs_file without having tweedledum installed. In #9754 the detection of tweedledum was updated to avoid import time detection so that the module can be imported even if tweedledum isn't installed. This was done through the use of the optionals decorators so that tweedledum is only attempted to be imported when the classicalfunction modules is used. However, the decorators don't wrap classmethod constructors by default and this caused the incorrect exception type to be raised. This commit fixes this by doing the runtime checking manually inside the from_dimacs_file constructor.

Details and comments

This commit fixes an oversight in the 0.24.0 release that caused an
accidental change in the exception raised when attempting to use
BooleanExpression.from_dimacs_file without having tweedledum installed.
In Qiskit#9754 the detection of tweedledum was updated to avoid import time
detection so that the module can be imported even if tweedledum isn't
installed. This was done through the use of the optionals decorators
so that tweedledum is only attempted to be imported when the
classicalfunction modules is used. However, the decorators don't wrap
classmethod constructors by default and this caused the incorrect
exception type to be raised. This commit fixes this by doing the runtime
checking manually inside the from_dimacs_file constructor.
@mtreinish mtreinish requested a review from a team as a code owner May 19, 2023 21:55
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@mtreinish mtreinish added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: Bugfix Include in the "Fixed" section of the changelog labels May 19, 2023
@coveralls
Copy link

Pull Request Test Coverage Report for Build 5028584711

  • 0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
  • 12 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.02%) to 85.923%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/circuit/classicalfunction/boolean_expression.py 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 93.76%
qiskit/extensions/quantum_initializer/squ.py 2 80.0%
crates/qasm2/src/lex.rs 3 91.39%
crates/qasm2/src/parse.rs 6 97.58%
Totals Coverage Status
Change from base Build 5026499530: 0.02%
Covered Lines: 71114
Relevant Lines: 82765

💛 - Coveralls

Copy link
Contributor

@Cryoris Cryoris left a comment

Choose a reason for hiding this comment

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

LGTM

@Cryoris Cryoris added this pull request to the merge queue May 22, 2023
Merged via the queue into Qiskit:main with commit 9f647c7 May 22, 2023
mergify bot pushed a commit that referenced this pull request May 22, 2023
#10132)

This commit fixes an oversight in the 0.24.0 release that caused an
accidental change in the exception raised when attempting to use
BooleanExpression.from_dimacs_file without having tweedledum installed.
In #9754 the detection of tweedledum was updated to avoid import time
detection so that the module can be imported even if tweedledum isn't
installed. This was done through the use of the optionals decorators
so that tweedledum is only attempted to be imported when the
classicalfunction modules is used. However, the decorators don't wrap
classmethod constructors by default and this caused the incorrect
exception type to be raised. This commit fixes this by doing the runtime
checking manually inside the from_dimacs_file constructor.

(cherry picked from commit 9f647c7)
@mtreinish mtreinish deleted the dimacs-error branch May 22, 2023 11:38
jakelishman pushed a commit that referenced this pull request May 22, 2023
#10132) (#10137)

This commit fixes an oversight in the 0.24.0 release that caused an
accidental change in the exception raised when attempting to use
BooleanExpression.from_dimacs_file without having tweedledum installed.
In #9754 the detection of tweedledum was updated to avoid import time
detection so that the module can be imported even if tweedledum isn't
installed. This was done through the use of the optionals decorators
so that tweedledum is only attempted to be imported when the
classicalfunction modules is used. However, the decorators don't wrap
classmethod constructors by default and this caused the incorrect
exception type to be raised. This commit fixes this by doing the runtime
checking manually inside the from_dimacs_file constructor.

(cherry picked from commit 9f647c7)

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
king-p3nguin pushed a commit to king-p3nguin/qiskit-terra that referenced this pull request May 22, 2023
Qiskit#10132)

This commit fixes an oversight in the 0.24.0 release that caused an
accidental change in the exception raised when attempting to use
BooleanExpression.from_dimacs_file without having tweedledum installed.
In Qiskit#9754 the detection of tweedledum was updated to avoid import time
detection so that the module can be imported even if tweedledum isn't
installed. This was done through the use of the optionals decorators
so that tweedledum is only attempted to be imported when the
classicalfunction modules is used. However, the decorators don't wrap
classmethod constructors by default and this caused the incorrect
exception type to be raised. This commit fixes this by doing the runtime
checking manually inside the from_dimacs_file constructor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the 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.

4 participants