Skip to content

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

Merged
merged 6 commits into from
Apr 16, 2017
Merged

Update svg_tooltip.py #8324

merged 6 commits into from
Apr 16, 2017

Conversation

DaveL17
Copy link
Contributor

@DaveL17 DaveL17 commented Mar 18, 2017

Original example supported a maximum of three patches. Proposed change should support any number of patches.

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')
Copy link
Member

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?

Copy link
Contributor Author

@DaveL17 DaveL17 Mar 19, 2017

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.

Copy link
Member

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.

Copy link
Contributor Author

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))
Copy link
Member

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(.

Copy link
Contributor Author

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):
Copy link
Member

Choose a reason for hiding this comment

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

y appears unused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed.

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

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):
Copy link
Member

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)):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed.

@dstansby
Copy link
Member

(fixes #8316)

@DaveL17
Copy link
Contributor Author

DaveL17 commented Mar 21, 2017

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 obj.id.slice(-1) from the original example script.

DaveL17 added 2 commits March 21, 2017 20:27
- 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):
Copy link
Member

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.

Copy link
Contributor Author

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.
Copy link
Member

@QuLogic QuLogic left a 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
Copy link
Member

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
Copy link
Member

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?

@dstansby dstansby added this to the 2.0.1 (next bug fix release) milestone Mar 28, 2017
@QuLogic QuLogic changed the title Update svg_tooltip.py [MRG+1] Update svg_tooltip.py Apr 15, 2017
@tacaswell tacaswell merged commit 565770a into matplotlib:master Apr 16, 2017
@tacaswell
Copy link
Member

Thanks @DaveL17 🎉 Hopefully we will here from you again soon!

tacaswell added a commit that referenced this pull request Apr 16, 2017
DOC: Update svg_tooltip.py
@tacaswell
Copy link
Member

backported to v2.0.x as 41bd991

@QuLogic QuLogic changed the title [MRG+1] Update svg_tooltip.py Update svg_tooltip.py Apr 17, 2017
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.

5 participants