Skip to content

Handle instructions without label in mpl drawer#4389

Closed
mtreinish wants to merge 12 commits intoQiskit:masterfrom
mtreinish:fix-mpl-label-draw
Closed

Handle instructions without label in mpl drawer#4389
mtreinish wants to merge 12 commits intoQiskit:masterfrom
mtreinish:fix-mpl-label-draw

Conversation

@mtreinish
Copy link
Copy Markdown
Member

Summary

Currently the mpl drawer is unconditionally accessing a label attribute
for several classes of instructions. However, it is not guaranteed that
a label is set for these instructions. This can cause a failure when
going to draw a circuit with these instructions if the label doesn't
exist. This commit fixes the issue by using a getattr and fixing the
fallback behavior to one that actually works.

Details and comments

Currently the mpl drawer is unconditionally accessing a label attribute
for several classes of instructions. However, it is not guaranteed that
a label is set for these instructions. This can cause a failure when
going to draw a circuit with these instructions if the label doesn't
exist. This commit fixes the issue by using a getattr and fixing the
fallback behavior to one that actually works.
Comment thread qiskit/visualization/matplotlib.py Outdated
@1ucian0 1ucian0 added the Changelog: Fixed Add a "Fixed" entry in the GitHub Release changelog. label May 21, 2020
1ucian0
1ucian0 previously approved these changes May 21, 2020
@1ucian0 1ucian0 self-requested a review May 21, 2020 15:11
@1ucian0 1ucian0 dismissed their stale review May 21, 2020 15:15

oh.. it's not working yet

@1ucian0
Copy link
Copy Markdown
Member

1ucian0 commented Jun 12, 2020

MPL circuit drawer issues are piling up, since we dont have a good way to track changes on it. I'm putting this PR on hold (meaning "do not merge yet") until #4544 (or a similar solution) gets in.

@1ucian0 1ucian0 added the on hold Can not fix yet label Jun 12, 2020
@1ucian0
Copy link
Copy Markdown
Member

1ucian0 commented Jun 22, 2020

#4544 is merged... Let's add a test with the new method!

I will try helping, but fill free to do it yourself and ask me if any question.

@1ucian0 1ucian0 marked this pull request as draft June 22, 2020 19:01
@1ucian0 1ucian0 removed the on hold Can not fix yet label Jun 22, 2020
@ajavadia ajavadia added this to the 0.15 milestone Jun 24, 2020
@mtreinish mtreinish marked this pull request as ready for review June 30, 2020 18:18
@mtreinish mtreinish requested a review from a team as a code owner June 30, 2020 18:18
@mtreinish
Copy link
Copy Markdown
Member Author

Sorry for the delay, this should be ready to review now

@ajavadia
Copy link
Copy Markdown
Member

ajavadia commented Jul 5, 2020

this seems already done via #4616

@mtreinish
Copy link
Copy Markdown
Member Author

Yeah, the bug that this was fixing is indeed closed by #4616. I'm not able to reproduce hard failures. It looks like it introduced other issues with the drawer, like the circuit names are being unconditionally treated as latex strings now which is different from expectations and causes weird formatting for some library elements. But we can open separate issues and fix those in a followup

@mtreinish mtreinish closed this Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: Fixed Add a "Fixed" entry in the GitHub Release changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AttributeError when drawing circuit using Matplotlib

4 participants