Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix manual contour label positions on sparse contours #1865

Merged
merged 8 commits into from

5 participants

@rephorm

Prior to this, contour labels were positioned at the nearest contour
vertex, which in some cases (e.g., straight contours) could be quite
distant from the desired location (and occasionally not even on the
correct contour).

This centers the label on the closest point on contour itself (using
linear interpolation between vertices).

Old:
contour_test old

New:
contour_test patched

Brian Mattern Fix manual contour label positions on sparse contours
Prior to this, contour labels were positioned at the nearest contour
vertex, which in some cases (e.g., straight contours) could be quite
distant from the desired location.

This centers the label on  the closest point on contour itself (using
linear interpolation between vertices).
283f358
@rephorm

In the images above, the blue dots are the coordinates passed to the manual option of the clabel function.

The script to generate these images is:

import numpy as np
from matplotlib import pyplot as pt
import matplotlib as mpl

pt.figure(figsize=(6,2))

x,y = np.meshgrid(np.arange(0,10), np.arange(0,10))
z = np.max(np.dstack([abs(x),abs(y)]), 2)
cs = pt.contour(x,y,z)
pts = np.array([(1.5,3.0), (1.5,4.4), (1.5,6.0)])
pt.clabel(cs, manual=pts)
pt.scatter(pts[:,0], pts[:,1])

pt.savefig("contour_test.png")
@pelson
Collaborator

This is a nice piece of work. We're trying to clean up the code to PEP8 standards, so would you mind doing things like putting spaces after commas + two new lines after main level objects etc. etc.?

Also, I appreciate that this is for manual contour label positioning, but can you conceive of an automatable test for this code?

Finally, would you be able to add a section to the whats_new.rst file in the documentation.

Thanks @rephorm

@rephorm

I've updated both my new code and the other code in contour.py to remove any warnings the pep8 script gives, and added a section to the whats_new.rst file.
Unfortunately, I'm not sure what the best way to automate testing this is.

@WeatherGod
Collaborator

Correct me if I am wrong, but wouldn't this patch also improve things for the automatic contour labeling (which has always been a pain)? If so, maybe we could find a degenerate case where the automatic contour labeling would be fixed by this and use that as a test?

@rephorm

You can supply a list of points to the manual parameter, so setting up the test isn't too hard. I'm just not sure what the right metric for the test is. If you just want to require pixel-identical output for the given input in the future, then that wouldn't be hard to do.

@WeatherGod
Collaborator

Ah, that would work too. Just simply utilize the regular image comparison framework that we have for everything else to produce the image in the example you gave originally. The image comparisons are designed to have a little bit of tolerance, and should be able to fail on the "before" image. We should make sure that it fails on the old behavior, of course.

@rephorm

I added a simple test based on the script above.

@pelson pelson commented on the diff
lib/matplotlib/tests/test_contour.py
@@ -137,3 +137,15 @@ def test_contour_shape_invalid_2():
ax.contour(x, y, z)
except TypeError as exc:
assert exc.args[0] == 'Input z must be a 2D array.'
+
+
+@image_comparison(baseline_images=['contour_manual_labels'])
@pelson Collaborator
pelson added a note

contour_manual_labels_pdf.png and contour_manual_labels_svg.png are re-createable and do not need adding (I think there is a lack of documentation regarding this, so I can see why you've done it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@pelson pelson commented on the diff
lib/matplotlib/tests/test_contour.py
@@ -137,3 +137,15 @@ def test_contour_shape_invalid_2():
ax.contour(x, y, z)
except TypeError as exc:
assert exc.args[0] == 'Input z must be a 2D array.'
+
+
+@image_comparison(baseline_images=['contour_manual_labels'])
+def test_contour_manual_labels():
+
+ x, y = np.meshgrid(np.arange(0, 10), np.arange(0, 10))
+ z = np.max(np.dstack([abs(x), abs(y)]), 2)
+
+ plt.figure(figsize=(6, 2))
+ cs = plt.contour(x,y,z)
+ pts = np.array([(1.5, 3.0), (1.5, 4.4), (1.5, 6.0)])
@pelson Collaborator
pelson added a note

Great test. Thanks for producing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rephorm

Yeah. The docs at http://matplotlib.org/devel/testing.html say to copy everything from result_images/test_foo/.
This should be updated.

@mdboom
Owner

@rephorm: I've created #1870 (and self-assigned) in reference to the docs.

@rephorm

Thanks, @mdboom
@pelson anything else you'd like to see done before this can be merged?

lib/matplotlib/contour.py
@@ -674,6 +686,64 @@ def labels(self, inline, inline_spacing):
paths.extend(additions)
+def _find_closest_point_on_leg(p1, p2, p0):
+ '''find closest point to p0 on line segment connecting p1 and p2'''
@dmcdougall Collaborator

Can you make these triple double quotes, """?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/matplotlib/contour.py
((15 lines not shown))
+ # project on to line segment to find closest point
+ proj = np.dot(d01, d21) / np.dot(d21, d21)
+ if proj < 0:
+ proj = 0
+ if proj > 1:
+ proj = 1
+ pc = p1 + proj * d21
+
+ # find squared distance
+ d = np.sum((pc-p0)**2)
+
+ return d, pc
+
+
+def _find_closest_point_on_path(lc, point):
+ '''
@dmcdougall Collaborator

And here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/matplotlib/contour.py
((18 lines not shown))
+ proj = 0
+ if proj > 1:
+ proj = 1
+ pc = p1 + proj * d21
+
+ # find squared distance
+ d = np.sum((pc-p0)**2)
+
+ return d, pc
+
+
+def _find_closest_point_on_path(lc, point):
+ '''
+ lc: coordinates of vertices
+ point: coordinates of test point
+ '''
@dmcdougall Collaborator

Ditto.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/matplotlib/contour.py
((24 lines not shown))
+ d = np.sum((pc-p0)**2)
+
+ return d, pc
+
+
+def _find_closest_point_on_path(lc, point):
+ '''
+ lc: coordinates of vertices
+ point: coordinates of test point
+ '''
+
+ # find index of closest vertex for this segment
+ ds = np.sum((lc - point[None, :])**2, 1)
+ imin = np.argmin(ds)
+
+ dmin = 1e10
@dmcdougall Collaborator

Should we use numpy's infinity here, just to be safe?

>>> import numpy as np
>>> 1e10 < np.inf
True
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dmcdougall
Collaborator

@rephorm This is a fantastic piece of work. Thanks for the patch!

@rephorm

@dmcdougall Both of those choices were merely for consistency with existing code. I've update the entire file to use triple-double quotes and np.inf as suggested.

@mdboom
Owner

@rephorn: Coding standards a bit new to matplotlib relative to its age, so there's a bit of a frustrating "do what I say, not what I do" feeling to it at the moment. It's slowly getting worked through, though. ;)

@dmcdougall
Collaborator

@rephorm You did the right thing. As @mdboom points out, we are in a transitional phase of making our codebase PEP8 compliant, and since I noticed the triple single quotes I thought it'd be better to nip it in the bud. Thanks for propagating the change over the whole file, it's much appreciated.

Anybody have any reservations? If not I'll push the shiny green button.

@rephorm

@dmcdougall, @mdboom I understand. I just figured I'd explain myself. Glad to hear things are becoming more standardized!

@mdboom
Owner

Looks good to me!

@dmcdougall dmcdougall merged commit 32bed39 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 28, 2013
  1. Fix manual contour label positions on sparse contours

    Brian Mattern authored
    Prior to this, contour labels were positioned at the nearest contour
    vertex, which in some cases (e.g., straight contours) could be quite
    distant from the desired location.
    
    This centers the label on  the closest point on contour itself (using
    linear interpolation between vertices).
  2. pep-8 compliance for commit 283f358

    Brian Mattern authored
  3. pep-8 compliance for rest of contour.py

    Brian Mattern authored
  4. add simple test for contour label positioning

    Brian Mattern authored
Commits on Mar 29, 2013
  1. remove uneeded files

    Brian Mattern authored
Commits on Apr 1, 2013
  1. use """ everywhere instead of '''

    Brian Mattern authored
  2. use np.inf instead of arbitrary 1e10

    Brian Mattern authored
Something went wrong with that request. Please try again.