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 iqp the matplotlib default style #11536

Merged
merged 16 commits into from
Jan 15, 2024
Merged

Make iqp the matplotlib default style #11536

merged 16 commits into from
Jan 15, 2024

Conversation

Cryoris
Copy link
Contributor

@Cryoris Cryoris commented Jan 10, 2024

Summary

Following #10950, this PR sets "iqp" as the new default for the matplotlib drawer.

Note: Due to the apparent impossibility of reproducing the precise matplotlib drawings locally I'm abusing the CI to generate the correct images, hence, opening this early draft PR.

Details and comments

In addition, this PR includes some additional changes

  • Using the black-white scheme must explicitly use style="bw" instead of style=False. This is a suggestion for a more consistent interface, the deprecation would be out into 0.46.
  • Refactors the DefaultStyle to not hardcode the default colors but load it from a JSON, such as all other color schemes.
  • Introduces a StyleDict, which is a dictionary handling matplotlib-specific aliases (such as "ec" for "edgecolor") instead of keeping a dictionary with duplicate entries.

* set IQP as default
* delete IQX as name
* remove "False" for bw
* refactor default style dict
@coveralls
Copy link

coveralls commented Jan 10, 2024

Pull Request Test Coverage Report for Build 7528651301

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 89.361%

Totals Coverage Status
Change from base Build 7503395201: 0.01%
Covered Lines: 59669
Relevant Lines: 66773

💛 - Coveralls

@Cryoris Cryoris added this to the 1.0.0 milestone Jan 12, 2024
@Cryoris Cryoris marked this pull request as ready for review January 12, 2024 14:31
@Cryoris Cryoris requested review from nonhermitian and a team as code owners January 12, 2024 14:31
@qiskit-bot
Copy link
Collaborator

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

@Cryoris Cryoris added Changelog: Removal Include in the Removed section of the changelog mod: visualization qiskit.visualization labels Jan 12, 2024
Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

LGTM, but there are still a few lint complains to fix. I think that the docs failures might get resolved after merging from main (at least that was the case for a PR I had with the same issues.

@jakelishman
Copy link
Member

I think the docs failure is local to this PR - the actual errors are these:

/home/vsts/work/1/s/.tox/docs/lib/python3.9/site-packages/qiskit/circuit/quantumcircuit.py:docstring of qiskit.circuit.quantumcircuit.QuantumCircuit.draw:30: ERROR: Unexpected indentation.
/home/vsts/work/1/s/.tox/docs/lib/python3.9/site-packages/qiskit/circuit/quantumcircuit.py:docstring of qiskit.circuit.quantumcircuit.QuantumCircuit.draw:31: WARNING: Block quote ends without a blank line; unexpected unindent.
/home/vsts/work/1/s/.tox/docs/lib/python3.9/site-packages/qiskit/visualization/circuit/circuit_visualization.py:docstring of qiskit.visualization.circuit.circuit_visualization.circuit_drawer:30: ERROR: Unexpected indentation.
/home/vsts/work/1/s/.tox/docs/lib/python3.9/site-packages/qiskit/visualization/circuit/circuit_visualization.py:docstring of qiskit.visualization.circuit.circuit_visualization.circuit_drawer:31: WARNING: Block quote ends without a blank line; unexpected unindent.

They're both probably because a bullet list is starting on the first continuation line - it likely wants a blank line before (and maybe after) the start of the list in the style argument.

ElePT
ElePT previously approved these changes Jan 15, 2024
Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

Approving in hopes that lint passes this time :)

@ElePT ElePT enabled auto-merge January 15, 2024 11:00
Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

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

Preliminary tests passing! I'll re-approve and hope for no further surprises.

@ElePT ElePT added this pull request to the merge queue Jan 15, 2024
Merged via the queue into Qiskit:main with commit 073d49d Jan 15, 2024
13 checks passed
ShellyGarion pushed a commit to ShellyGarion/qiskit-terra that referenced this pull request Jan 18, 2024
* First try

* set IQP as default
* delete IQX as name
* remove "False" for bw
* refactor default style dict

* Missing future annotations, revert img threshold

* lint

* Update ref images and fix dict loading

some img will still need updating

* Fix loading of styles via dict

* Fix usage of stylename and iqx usage

* Fix default font ratio

* Update creg_initial_true img

* cleanup

* add reno

* Attempt 1 to gain Sphinx' goodwill

* thou shalt make lint

* thou shan't change default values

---------

Co-authored-by: Elena Peña Tapia <57907331+ElePT@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Removal Include in the Removed section of the changelog mod: visualization qiskit.visualization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants