Skip to content

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

Closed

Description

@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:
test1 4 2

In matplotlib v1.4.3 the plot looks like this:
test1 4 3

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

Activity

  1. added
    API: changesChanges to the public API, typically requiring deprecation.
    on Mar 28, 2015
  2. tacaswell commented on Mar 28, 2015

    @tacaswell
    Member

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

  3. efiring commented on Mar 28, 2015

    @efiring
    Member

    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.

  4. breedlun commented on Mar 28, 2015

    @breedlun
    ContributorAuthor

    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:
    test1 3 1

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

  5. added this to the milestone on Mar 28, 2015
  6. tacaswell commented on Mar 30, 2015

    @tacaswell
    Member

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

    Sorry, I was not careful about this twice.

  7. tacaswell commented on Mar 30, 2015

    @tacaswell
    Member

    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.

  8. efiring commented on Mar 30, 2015

    @efiring
    Member

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

  9. tacaswell commented on Mar 30, 2015

    @tacaswell
    Member

    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?).

  10. breedlun commented on Mar 30, 2015

    @breedlun
    ContributorAuthor

    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.

  11. tacaswell commented on Mar 30, 2015

    @tacaswell
    Member

    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 [email protected]
    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)
    .

  12. breedlun commented on Mar 30, 2015

    @breedlun
    ContributorAuthor

    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?

  13. efiring commented on Mar 30, 2015

    @efiring
    Member

    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.

  14. tacaswell commented on Mar 30, 2015

    @tacaswell
    Member

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

  15. breedlun commented on Mar 30, 2015

    @breedlun
    ContributorAuthor

    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.

  16. 1 remaining item

  17. added
    Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
    on Jun 16, 2015
  18. modified the milestones: , on Jun 16, 2015
  19. tacaswell commented on Jun 16, 2015

    @tacaswell
    Member

    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 [email protected]
    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)
    .

  20. tacaswell commented on Jul 22, 2015

    @tacaswell
    Member

    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

  21. efiring commented on Jul 22, 2015

    @efiring
    Member

    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.

  22. breedlun commented on Jul 22, 2015

    @breedlun
    ContributorAuthor

    Option 1 works for me.

  23. myshen commented on Jul 22, 2015

    @myshen
    Contributor

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

  24. tacaswell commented on Jul 31, 2015

    @tacaswell
    Member

    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.

  25. added a commit that references this issue on Jul 31, 2015
  26. self-assigned this
    on Jul 31, 2015
  27. efiring commented on Aug 1, 2015

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

Metadata

Metadata

Assignees

Labels

API: changesChanges to the public API, typically requiring deprecation.Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions