-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Update svg_tooltip.py #8324
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
Update svg_tooltip.py #8324
Conversation
Original example supported a maximum of three patches. Proposed change should support any number of patches.
# Set id for the annotations | ||
for i, t in enumerate(ax.texts): | ||
t.set_gid('tooltip_%d' % i) | ||
rect1 = plt.Rectangle((10, -20), 10, 5, fc='blue') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change from the Circle
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't able to find a clean way to iterate the coordinates of a circle in the example for loop. That's the only reason. Two rectangles made the example straightforward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Using a CirclePolygon
, you can use the .xy
property of both, but that requires an additional import from matplotlib.patches
and it's not really a perfect circle.
With Circle
, you can use the .center
property, but then they're not exactly the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. For the example, it didn't seem terribly pertinent that the shapes had to be different (I reckon other than to show that it works with different types of patches.)
annotate = ax.annotate(labels[i], xy=item.get_xy(), xytext=(0, 0), | ||
textcoords='offset points', color='w', ha='center', | ||
fontsize=8, bbox=dict(boxstyle='round, pad=.5', fc=(.1, .1, .1, .92), | ||
ec=(1., 1., 1.), lw=1, zorder=1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be aligned with dict(
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
el = xmlid['patch_%d' % i] | ||
el.set('onmouseover', "ShowTooltip(this)") | ||
el.set('onmouseout', "HideTooltip(this)") | ||
for i, y in enumerate(shapes): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
y
appears unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hang on a sec. It appears that tooltip = xmlid['mytooltip_%d' % i]
throws a TypeError if y is not used.
Disagreed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're doing something wrong then; y
should not be necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed this concern in the latest commit. I think all identified issues have been cleared.
shapes = [rect1, rect2] | ||
labels = ['This is a blue rectangle.', 'This is a green rectangle'] | ||
|
||
for i, item in enumerate(shapes): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for i, (item, label) in enumerate(zip(shapes, labels)):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
(fixes #8316) |
Just a note that the script works while patches are 10 or less. When the 11th patch is applied, the SVG will display the tooltip associated with the first patch. I suspect it's the |
- Last commit would fail on the 11th patch. This commit should support an "unlimited" number of patches. - Updates for {} formatting. - Edits save file location to remove link to user-specific path erroneously included in last commit.
Addresses the 'y appears unused' issue.
el = xmlid['patch_%d' % i] | ||
el.set('onmouseover', "ShowTooltip(this)") | ||
el.set('onmouseout', "HideTooltip(this)") | ||
for i in enumerate(shapes): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for i in range(len(shapes))
; don't need the enumerate
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Commit now references index instead.
References the index of the shape object instead of the enumeration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to rebase and squash.
@@ -1,3 +1,6 @@ | |||
#! /usr/bin/env python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not executable, so I don't think we need this (and I don't think it should be made executable either.)
@@ -1,3 +1,6 @@ | |||
#! /usr/bin/env python | |||
# -*- coding: utf-8 -*- | |||
|
|||
""" | |||
SVG tooltip example |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a row of ===
above this to make the title sphinx-gallery compliant?
Thanks @DaveL17 🎉 Hopefully we will here from you again soon! |
backported to v2.0.x as 41bd991 |
Original example supported a maximum of three patches. Proposed change should support any number of patches.