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

output as first kwarg in circuit.draw #3632

Merged
merged 6 commits into from Dec 20, 2019
Merged

output as first kwarg in circuit.draw #3632

merged 6 commits into from Dec 20, 2019

Conversation

nonhermitian
Copy link
Contributor

Summary

Makes it so one can just do qc.draw('mpl') verses always having to write qc.draw(output='mpl')

Details and comments

@nonhermitian nonhermitian added the type: enhancement It's working, but needs polishing label Dec 19, 2019
@nonhermitian nonhermitian added this to To do in Visualization and Juypter Tools via automation Dec 19, 2019
@1ucian0
Copy link
Member

1ucian0 commented Dec 19, 2019

Fully support this! However, it's an API change and we should deprecate the first positional argument as an integer.

@nonhermitian
Copy link
Contributor Author

I am not sure if we really need a depreciation warning here as I have never seen anyone use a bare call to draw to set the scale (I have not actually seen anyone set the scale outside of the tutorials showing that it is possible).

@kdk
Copy link
Member

kdk commented Dec 19, 2019

In general, we want to deprecated old behavior, even if we think it was rarely used. However, correctly deprecating changes in function argument order is quite difficult. (I wrote a decorator for changing argument names for #3399 that could maybe be expanded for this purpose, but its still a WIP.)

Maybe in this case, the right balance is to change the order, and then check if output is numeric, and if so, raise a more descriptive error message about the API change. Thoughts?

@kdk kdk merged commit 5cd120d into Qiskit:master Dec 20, 2019
Visualization and Juypter Tools automation moved this from To do to Done Dec 20, 2019
@kdk kdk added Changelog: API Change Include in the "Changed" section of the changelog Changelog: Deprecation Include in "Deprecated" section of changelog labels Dec 20, 2019
faisaldebouni pushed a commit to faisaldebouni/qiskit-terra that referenced this pull request Aug 5, 2020
* output as first kwarg

* add depreciation warning.

* fix import order

* add depreciation
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 Changelog: Deprecation Include in "Deprecated" section of changelog type: enhancement It's working, but needs polishing
Development

Successfully merging this pull request may close these issues.

None yet

3 participants