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

override control of standard gates where possible #3631

Merged
merged 27 commits into from
Jan 30, 2020

Conversation

ewinston
Copy link
Contributor

Summary

Attempt to override the control method of standard gates which do not cause a cyclic import. This simplifies the logic somewhat in add_control.py.

Details and comments

This is based off a discussion which occurred in pr #3600 . This pr avoids cyclic import warnings raised by pylint by moving predefined controlled gates into their base modules. This change may break code which imports some controlled gates.

@ajavadia ajavadia added the Changelog: API Change Include in the "Changed" section of the changelog label Dec 19, 2019
qiskit/extensions/standard/rx.py Show resolved Hide resolved
qiskit/circuit/add_control.py Outdated Show resolved Hide resolved
@kdk kdk self-assigned this Jan 8, 2020
@kdk kdk assigned ewinston and unassigned kdk Jan 15, 2020
@ewinston
Copy link
Contributor Author

There seems to be a single error which only occurs when running multiple environments with tox that does not seem to occur with e.g. make test or tox -py37.

@kdk
Copy link
Member

kdk commented Jan 17, 2020

There seems to be a single error which only occurs when running multiple environments with tox that does not seem to occur with e.g. make test or tox -py37.

This could be due to ToffoliGate being defined in both x.py and ccx.py. With this PR, can we remove ccx.py?

@ewinston
Copy link
Contributor Author

ewinston commented Jan 17, 2020

@kdk I kept ccx.py, and other controlled gate modules, for deprecation purposes. Your point, however, helped me to find a conflicting import reference for ToffoliGate which I fixed. This seems to have solved the bug. Thanks!

Also, I removed redundant definitions in to be deprecated gate modules.

qiskit/extensions/standard/cz.py Outdated Show resolved Hide resolved
qiskit/extensions/standard/h.py Show resolved Hide resolved
kdk
kdk previously approved these changes Jan 27, 2020
@kdk kdk added the automerge label Jan 27, 2020
@kdk kdk merged commit 592c898 into Qiskit:master Jan 30, 2020
@ewinston ewinston deleted the gateconsolidate branch June 9, 2020 19:52
faisaldebouni pushed a commit to faisaldebouni/qiskit-terra that referenced this pull request Aug 5, 2020
* move controlled gate class definitions into base gate module.

* fix cyclic imports

* update for new crx and cry

* linting

* linting

* linting

* minor

* wrong import of U3Gate

* module level pylint cyclic-import disable

* update mapper ground truth

* hack to get tox to work

* solve bug associated with ToffoliGate import.

* avoid multiple definitions

* update warning category.

* change stack level

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants