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

Option to skip drawing wire labels #6410

Merged
merged 12 commits into from
Oct 21, 2024
Merged

Option to skip drawing wire labels #6410

merged 12 commits into from
Oct 21, 2024

Conversation

dwierichs
Copy link
Contributor

Context:
Wire labels are drawn by default.

Description of the Change:
This PR adds an option to hide the wire labels, which can be useful to produce more clean images, for example.

Benefits:
A convenient feature.

Possible Drawbacks:
More options, I suppose.

Related GitHub Issues:

Copy link
Contributor

@paul0403 paul0403 left a comment

Choose a reason for hiding this comment

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

Thanks for adding this! 💯

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.

So it's show_wire_labels for tape_text but label_options for tape_mpl?

@dwierichs
Copy link
Contributor Author

dwierichs commented Oct 18, 2024

So it's show_wire_labels for tape_text but label_options for tape_mpl?

Yeah I don't love that. To me, it feels canonical to embed this new option in the label_options kwarg of tape_mpl, which tape_text does not have, though.
An alternative would be to add show_wire_labels to tape_mpl as well, which I don't mind.
The signatures differ anyways already, so I don't know how to decide this.

I tried adding show_wire_labels to tape_mpl instead, and I think that's better, also because we don't hijack the data type of label_options.

@dwierichs dwierichs requested review from paul0403 and albi3ro October 18, 2024 07:57
@dwierichs dwierichs marked this pull request as ready for review October 18, 2024 08:00
Copy link

codecov bot commented Oct 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.38%. Comparing base (ade6465) to head (bcaeb1c).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6410      +/-   ##
==========================================
- Coverage   99.70%   99.38%   -0.32%     
==========================================
  Files         447      447              
  Lines       42424    42434      +10     
==========================================
- Hits        42299    42174     -125     
- Misses        125      260     +135     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

👍

@dwierichs dwierichs enabled auto-merge (squash) October 21, 2024 08:33
@albi3ro albi3ro disabled auto-merge October 21, 2024 14:27
@albi3ro albi3ro merged commit 2c2fd8a into master Oct 21, 2024
39 of 40 checks passed
@albi3ro albi3ro deleted the draw-no-labels branch October 21, 2024 14:27
mudit2812 pushed a commit that referenced this pull request Nov 11, 2024
**Context:**
Wire labels are drawn by default.

**Description of the Change:**
This PR adds an option to hide the wire labels, which can be useful to
produce more clean images, for example.

**Benefits:**
A convenient feature.

**Possible Drawbacks:**
More options, I suppose.

**Related GitHub Issues:**

---------

Co-authored-by: Christina Lee <christina@xanadu.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants