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

make three qubit decomposition tests scipy version dependent #3726

Merged
merged 8 commits into from Feb 2, 2021

Conversation

balopat
Copy link
Contributor

@balopat balopat commented Feb 1, 2021

This is a fix for older environments. The code already does check whether scipy has the cossin method. This PR just makes this explicit in the tests as well - by skipping those tests that would require cossin to exist. I also added a test for when scipy is <1.5.0 - in which case we except an ImportError. This latter test is not currently tested in our CI (see #3552 for suggestion about having maybe an upper / lower limit testing for dependencies).

cc @dstrain115

@balopat balopat requested review from cduck, vtomole and a team as code owners February 1, 2021 19:35
@google-cla google-cla bot added the cla: yes Makes googlebot stop complaining. label Feb 1, 2021
def decorator(func):
try:
# the cossin function was introduced in 1.5
from scipy.linalg import cossin
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to tell pylint to ignore unused import.

cirq/optimizers/three_qubit_decomposition_test.py Outdated Show resolved Hide resolved
@balopat balopat added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Feb 1, 2021
@CirqBot
Copy link
Collaborator

CirqBot commented Feb 1, 2021

Automerge cancelled: A status check is failing.

@CirqBot CirqBot removed the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Feb 1, 2021
@balopat balopat added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Feb 1, 2021
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Feb 1, 2021
@CirqBot
Copy link
Collaborator

CirqBot commented Feb 1, 2021

Automerge cancelled: A status check is failing.

@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Feb 1, 2021
@balopat balopat added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Feb 2, 2021
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Feb 2, 2021
@CirqBot CirqBot merged commit 7eb27cf into quantumlib:master Feb 2, 2021
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Makes googlebot stop complaining.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants