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

Add Qiskit Textbook circuit drawer style #8147

Merged
merged 7 commits into from
Jun 8, 2022

Conversation

frankharkins
Copy link
Member

@frankharkins frankharkins commented Jun 7, 2022

Summary

We have been using a custom circuit drawer on the Qiskit Textbook for a while now. This PR makes it part of Qiskit itself to make it easier for users and contributors to reproduce our diagrams.

Screenshot 2022-06-08 at 09 26 03

Details and comments

Only list of styles I could see was in the qiskit.visualization.qcstyle.DefaultStyle docstring, which I updated.

Color scheme designed by Grace Lindsell and Russell Huffman.

@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

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

@HuangJunye
Copy link
Collaborator

Sounds great! I didn't know the textbook use a different style. Can you provide a few links to examples to see the style? It would be good to put a screenshot in the PR just for the record as well.

Copy link
Contributor

@enavarro51 enavarro51 left a comment

Choose a reason for hiding this comment

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

There are specific 'mpl' snapshot tests for "iqx", "bw", and "iqx-dark", so it would be a good idea to add one for "textbook" as well. You can use the "iqx" one as a starter. If there are specific gates that your style displays differently from "default" and are not on the "iqx", you should probably add them to the display. If you haven't done one before, there are guidelines here, https://github.com/Qiskit/qiskit-terra/blob/main/CONTRIBUTING.md#snapshot-testing-for-visualizations.

qiskit/visualization/qcstyle.py Outdated Show resolved Hide resolved
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

One American localisation comment, but other than that this looks fine to me - I wouldn't want us to get in the habit of supplying a lot of themes, but the very public ones we use in our own documentation are probably sensible.

@coveralls
Copy link

coveralls commented Jun 7, 2022

Pull Request Test Coverage Report for Build 2460622816

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.005%) to 84.411%

Files with Coverage Reduction New Missed Lines %
qiskit/pulse/library/waveform.py 3 89.36%
Totals Coverage Status
Change from base Build 2454743017: -0.005%
Covered Lines: 54666
Relevant Lines: 64762

💛 - Coveralls

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the test!

@jakelishman jakelishman added Changelog: New Feature Include in the "Added" section of the changelog automerge labels Jun 8, 2022
@jakelishman jakelishman added this to the 0.21 milestone Jun 8, 2022
@mergify mergify bot merged commit 792ff60 into Qiskit:main Jun 8, 2022
@frankharkins frankharkins deleted the textbook-style branch June 8, 2022 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants