BUGFIX: This change fixes #2475, and incorporates pull request #2806

[bwkeller]

I think this should handle both the changes from milli's #2806 pull request and my #2809 pull request.

[tacaswell]

@bwkeller As a side note, it is better form to work on feature branches, rather than directly on master. It makes resetting your master after this is merged easier.

[tacaswell]

I am going to close this because this way of fixing the problem changes behaviour of add_label_near from what is specified in the doc string (under this change passing in transform as a kwarg has no effect on the behaviour of the function).

[tacaswell]

Sorry, changed my mind about this.

[bwkeller]

@tacaswell Thanks for the advice, this is my first time contributing, so I'm probably making a bunch of rookie mistakes.

[bwkeller]

I think transform still changes the x,y points that are passed to add_label_near (lines 570-575). It just isn't used to generate xcmin, since find_nearest_contour() returns xmin,ymin in data coordinates (I believe), and so the transform kwarg shouldn't be used there, but instead the data transform needs to be used.

[tacaswell]

tl;dr this is the right fix

When you click on the graph the function contour.add_label_near is called via the call back installed by the BlockingContourLabeler called around L217 in contour.py. The call to add_label_near is in the callback function button1 around L321 in blocking_input.py (which traces back through two more classes and a bunch of event handling to get triggered, at this point I am going to assume it does what it looks like it does, which is trigger button1 when you click the left button). If you not the value of transfrom=False. Thus, in our function here transform is not None so we skip the first if statement (transform is still False) and then also skip the second if statement as well.

It is important to note that transforms are named for what they take to display coordinates (see http://matplotlib.org/users/transforms_tutorial.html), thus what passing in a value that is not False says "I am passing in data is a funny unit, please convert it to display units before continuing". The value None is taken to mean 'do the default' which in this case means to assume the data is in data units. In the case of the callback, the mouse events return locations in display units, hence nothing needs to be done and transform=False.

The function find_nearest_countour returns results/works in display units (hence the whole rig-a-marole above to make sure x and y are in display units). What is going on in #1865 is to add points along the contours so that the labels can be put into place, but looks like it was only tested using the labeler in a slightly different way where you pass it a list of points in data-space hence the branch of code which it is going through here was never tested. In both cases, the display space units for where to add the point on the contour need to be inverted to data space, hence this is the correct fix.

I wrote this out so I don't have to figure this out again.

I have cherry-picked your commits to be against v1.3.x and squashed your commits down to a single commit in PR #2843. I did this for two reasons: I wanted this to be on v1.3.x in case we do make a 1.3.2 release and so that I can easily add a test image to this to make sure this does not break again.

Sorry if you figured all of this out and I am just being dense.