10000 BUGFIX: This change fixes #2475, where contour labels added manually by tacaswell · Pull Request #2843 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

BUGFIX: This change fixes #2475, where contour labels added manually #2843

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.

Sign up for GitHub

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 4 commits into from
Jul 10, 2014
Merged

BUGFIX: This change fixes #2475, where contour labels added manually #2843

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5834957
BUGFIX: This change fixes #2475, where contour labels added manually
bwkeller Feb 13, 2014
1e0b084
TST : added test to exercise transform kwarg to `add_label_near`
tacaswell Feb 26, 2014
01da70d
DOC : improved add_label_near
tacaswell Jul 8, 2014
23b5240
DOC : cleaned up/clarified docstring
tacaswell Jul 9, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
BUGFIX: This change fixes #2475, where contour labels added manually 
produce ugly spikes in the contours.  The problem was that screen
coordinates were being passed as data coordinates when the contour was
split to add room for the label.

Incorporate my fix and miili's #2806 change
  • Loading branch information
@bwkeller @tacaswell
bwkeller authored and tacaswell committed Feb 21, 2014
commit 583495709acbf09500abc1e62bfc690c28903c03
5 changes: 1 addition & 4 deletions lib/matplotlib/contour.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,10 +576,7 @@ def add_label_near(self, x, y, inline=True, inline_spacing=5,
# be a vertex in the path. So, if it isn't, add a vertex here
paths = self.collections[conmin].get_paths()
Copy link
Member

Choose a reason for hiding this comment

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

what if the path object from the collection has a different transform than what we are expecting? Otherwise, this all makes sense, albeit it could use some more commenting

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the assumption is that the points in the path are in data space as iirc everything returned by find_nearest_contour is working in screen space. The bug was in the case were you wanted to specify the location of the label in screen-units, but when the location never got converted back to data-space units when adding a point to the path which is what generated the huge spikes.

I think the issue you are bring up is if points in the path are not specified in data units so instead of self.ax.transData.inverted() in should be paths[segmin].get_transform().inverted()? Presumably that transform should be passed back in on line 585/582 when the new path is created.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that is exactly what my concern is. The transforms framework can have a lot of redundant parts, which can mask incorrect usage. We shouldn't assume that we know the transform of the path object, that is known only by the path object itself, so we should use it here.

Admittedly, my brain does mini-explosions reading through some of these code parts because I easily lose track of the coder's intent and expectation.

lc = paths[segmin].vertices
if transform:
xcmin = transform.inverted().transform([xmin, ymin])
else:
xcmin = np.array([xmin, ymin])
xcmin = self.ax.transData.inverted().transform_point([xmin, ymin])
if not np.allclose(xcmin, lc[imin]):
lc = np.r_[lc[:imin], np.array(xcmin)[None, :], lc[imin:]]
paths[segmin] = mpath.Path(lc)
Expand Down
0