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

[unitaryhack] Request for new matplotlib drawing styles #2662

Merged
merged 20 commits into from
Jun 9, 2022
Merged

[unitaryhack] Request for new matplotlib drawing styles #2662

merged 20 commits into from
Jun 9, 2022

Conversation

trentfridey
Copy link
Contributor

@trentfridey trentfridey commented Jun 4, 2022

Context:
Adds styles requested in #2555

Description of the Change:

Adds the solarized light and dark styles to the available styles for drawer

Benefits:
Adds more options for clean and readable circuit diagram graphics

Related GitHub Issues:
#2555

@codecov
Copy link

codecov bot commented Jun 4, 2022

Codecov Report

Merging #2662 (75ac23b) into master (4b7b59b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2662   +/-   ##
=======================================
  Coverage   99.59%   99.59%           
=======================================
  Files         249      249           
  Lines       20256    20280   +24     
=======================================
+ Hits        20173    20197   +24     
  Misses         83       83           
Impacted Files Coverage Δ
pennylane/drawer/style.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b7b59b...75ac23b. Read the comment docs.

@trentfridey trentfridey marked this pull request as ready for review June 6, 2022 18:55
Copy link
Contributor

@albi3ro albi3ro left a comment

Choose a reason for hiding this comment

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

@trentfridey Thanks for the excellent contribution!

It looks like you've got all the code figured out, which is always nice to see.

Initially, I was going to ask for more contrast between the background and text, but upon viewing them in my dark-themed IDE, they looked wonderful.

One minor change that made the graphics much clearer was:

plt.rcParams['patch.linewidth'] = 3.0

I had to set the 'patch.linewidth' in the black_white style as well.

I'd also like to see how qml_drawer.rst looks with a 3x2 table instead of 5x1. The images start to get a bit too small with 5x1.

doc/code/qml_drawer.rst Outdated Show resolved Hide resolved
@albi3ro albi3ro self-requested a review June 7, 2022 21:31
tests/drawer/test_style.py Outdated Show resolved Hide resolved
Copy link
Contributor

@albi3ro albi3ro left a comment

Choose a reason for hiding this comment

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

Looks excellent!

Just need to run black -l 100 pennylane/drawer/style.py and I'm happy to approve +1

@albi3ro
Copy link
Contributor

albi3ro commented Jun 9, 2022

@trentfridey So close! but now it looks like black -l 100 needs to be run on a test now too. These minor things can be a bit of a nuisance, but they make things much easier in the long term.

Copy link
Contributor

@albi3ro albi3ro left a comment

Choose a reason for hiding this comment

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

Excellent!
Thanks for the great contribution 🎉

@albi3ro albi3ro merged commit bee93e4 into PennyLaneAI:master Jun 9, 2022
@albi3ro albi3ro added the unitaryhack-accepted This contribution has been accepted for a UnitaryHack issue label Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unitaryhack-accepted This contribution has been accepted for a UnitaryHack issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants