Annotation with negative axes fraction coordinate placed incorrectly with v1.4.3

[breedlun]

I am finding that annotations placed outside of axes get placed incorrectly in v1.4.3. Here is an example:

import matplotlib.pyplot as plt
import matplotlib as mpl

fig, ax = plt.subplots()

ax.annotate('+ pts', 
    xytext = [40, 20], textcoords = 'axes points', \
    xy = [40, 20], xycoords = 'axes points', fontsize = 32)
ax.annotate('- pts', 
    xytext = [40, -20], textcoords = 'axes points', \
    xy = [40, -20], xycoords = 'axes points', fontsize = 32)
ax.annotate('+ frac', 
    xytext = [0.5, 0.1], textcoords = 'axes fraction', \
    xy = [0.5, 0.1], xycoords = 'axes fraction', fontsize = 32)
ax.annotate('- frac', 
    xytext = [0.5, -0.1], textcoords = 'axes fraction', \
    xy = [0.5, -0.1], xycoords = 'axes fraction', fontsize = 32)
plt.savefig('test' + mpl.__version__ + '.png')

In matplotlib v1.4.2 the plot looks like this:

In matplotlib v1.4.3 the plot looks like this:

Apparently v1.4.3 takes a negative axes fraction (or points) and wraps it around to the top of the axes.

[tacaswell]

See #4019 This is un-breaking something that was broken in 1.4.0.

[efiring]

The 4.3 behavior seems very odd, though. Why would anyone want this behavior? It seems inconsistent with every other case of positioning--does any other object positioning function do a wraparound like this? It imposes unnecessary restrictions on where one can position annotations.

[breedlun]

Yes, I agree with efiring. I much prefer the v1.4.2 behavior.

I just tried out v1.3.1 and got the following plot:

The coordinates specified with axes points get wrapped around, while the coordinates specified with axes fractions behave as I would expect.

[tacaswell]

For reference, the original issue which triggered this 'fix' was #4012

Sorry, I was not careful about this twice.

[tacaswell]

I am also not sure what the best course of action here is.

I am in favor of rolling back to the 1.3.1 behavior for back-compatibility issues, but the asymmetry is annoying.

[efiring]

@tacaswell, I think this just points to the complexity of the code, and some uncertainty as to how it really should behave.
How about a more radical change: no wrapping at all. If something like the wrapping behavior is needed, e.g. to make it easy to position with respect to the top instead of the bottom, then this should be specified via an additional kwarg, not via the sign of the offset.

[tacaswell]

I think I am ok with that.

Do we want to just revert #4019 for now and add the extra kwargs is later or try to do both now?

This does put us on track to have kwargs which are only sometimes used which is part of the problem with all of the FancyArrow/Box code.

How about putting the logic in the textcoord value, ex 'axes fraction invert-x'? This of course opens up a different world of pain (as now we are doing fancy parsing on string values) but we are already most of the way there.

The other option I see is invert_x and invert_y boolean kwargs, but I am having trouble making sense of what those would mean in data units (default to None and raise if non-none and data units?).

[breedlun]

I am obviously not a lead matplotlib developer, but I thought I would put in my two cents as a user. I don't see why you would want wrapping in 99 % of the cases. IMHO, if a user wants wrapping behavior, they can easily implement it in their own code.

[tacaswell]

The compelling case is mostly for points based units. Saying "I want the
Annotation to be 15 pixels from the right edge of the axes" is very
difficult without wrapping of some sort provided by the library (as you
don't know until draw time how big (in pixels) the axes or figure will be).

On Sun, Mar 29, 2015 at 10:37 PM Benjamin Reedlunn notifications@github.com
wrote:

I am obviously not a lead matplotlib developer, but I thought I would put
in my two cents as a user. I don't see why you would want wrapping in 99 %
of the cases. IMHO, if a user wants wrapping behavior, they can easily
implement it in their own code.

—
Reply to this email directly or view it on GitHub
#4292 (comment)
.

[breedlun]

Hmm, I see. Can't you get the axes size in normalized figure coordinates, and then use the figure size to figure out the physical size of the axes? Perhaps you need a draw() somewhere in there…

Here is another thought. The OffsetFrom class allows users to specify an annotation position as an offset from a reference point on an existing annotation. What about allowing users to specify an annotation position as an offset from a reference point on a set of axes?

[efiring]

Exactly--what is needed is not wrapping, but an origin. I think it would be enough to be able to specify origins as "SW", "SE", "NE", and "NW". Positions would be signed offsets from those origins. No wrapping at all. No inversions.

[tacaswell]

@efiring I like that a lot better than either of my ideas.

[breedlun]

What about specifying the origin in normalized axes coordinates? That would combine the flexibility of the OffsetFrom class with the simplicity of just adding a kwarg to annotate.

[breedlun]

@tacaswell You just sent out an email to the dev list asking if there are any issues that we would like to see tagged as "next release critical". In my mind this is one of them. The ability to specify an origin would be great, but if it turns out to be difficult to implement, would it be possible to just roll back the 1.4.2 annotation behavior?

[tacaswell]

Rolling back to the 1.4.2 behaviour is my current base-line for this and I
view as critical bug fix (I have just not built up the courage to dive back
into this code recently)

Can you create a new issue for adding the origin. I am not sure you can't
currently get something like an arbitrary origin with enough use of
transforms.

On Tue, Jun 16, 2015 at 9:22 AM Benjamin Reedlunn notifications@github.com
wrote:

@tacaswell https://github.com/tacaswell You just sent out an email to
the dev list asking if there are any issues that we would like to see
tagged as "next release critical". In my mind this is one of them. The
ability to specify an origin would be great, but if it turns out to be
difficult to implement, would it be possible to just roll back the 1.4.2
annotation behavior?

—
Reply to this email directly or view it on GitHub
#4292 (comment)
.

[tacaswell]

Ok, so here is my understanding of what happened here.

For 1.4.0 I refactored annotations (#2351 ) but I missed that {'axes points', 'axes pixel', 'figure points', 'figure pixel'} were special cased to wrap. Presumably from the function name to maintain back compatibility. This was an unintentional and undocumented API break.

This was noticed in #4012 and fixed in #4019 but that catches too many of the coordinate systems (should not be all things that start with 'axes') so fixed one API, but broke others.

I see two reasonable courses of action:

1. revert back to 1.4.2 behavior with nothing wrapping.
2. revert back to 1.3.1 behavior with somethings wrapping.

I am inclined to go with option 1 and just accept the API break (and hope no one takes my commit rights away).

attn @efiring @stretch97 @QuLogic @myshen

[efiring]

Agreed: best course of action for now is to eliminate wrapping. That's good enough for this release. It makes the behavior simpler and more predictable. Then, with no time pressure, whoever is suitably inspired can put in a PR to add an origin kwarg. The full Annotation and even Text API's are so complicated that I think we can be allowed a little breakage in the process of trying to fix bugs and get the code and APIs under control. I took that point of view in #4178; for example, when eliminating the YAArrow code path I did not try to support every quirk and surprise of its behavior and API.

[breedlun]

Option 1 works for me.

[myshen]

+1 option 1
+1 origin @tacaswell's comment is exactly the use case prompting my "fix".

[tacaswell]

It has dawned on me that there is a very easy way to implement an 'origin' as annotation already takes two positions (xy, xyann). I think we can just allow the word 'offset' to be prefixed to any of the textcoords to mean 'use this unit relative to the xycoord'. It doesn't seem too bad from the user side, not sure how hard it would be to do on the code side.

[efiring]

It might be a good idea--but the annotation API and code are already so complicated and idiosyncratic that I think it would be a mistake to make them even more complicated right now. Actually, I probably don't understand exactly what you are suggesting here.