You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Annotation with negative axes fraction coordinate placed incorrectly with v1.4.3 #4292
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.
@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.
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?).
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.
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)
.
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?
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.
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.
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:
@tacaswellhttps://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)
.
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:
revert back to 1.4.2 behavior with nothing wrapping.
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).
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.
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.
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.
Activity
tacaswell commentedon Mar 28, 2015
See #4019 This is un-breaking something that was broken in 1.4.0.
efiring commentedon Mar 28, 2015
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 commentedon Mar 28, 2015
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 commentedon Mar 30, 2015
For reference, the original issue which triggered this 'fix' was #4012
Sorry, I was not careful about this twice.
tacaswell commentedon Mar 30, 2015
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 commentedon Mar 30, 2015
@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 commentedon Mar 30, 2015
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
textcoordvalue, 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_xandinvert_yboolean kwargs, but I am having trouble making sense of what those would mean in data units (default toNoneand raise if non-none and data units?).breedlun commentedon Mar 30, 2015
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 commentedon Mar 30, 2015
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:
breedlun commentedon Mar 30, 2015
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
OffsetFromclass 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 commentedon Mar 30, 2015
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 commentedon Mar 30, 2015
@efiring I like that a lot better than either of my ideas.
breedlun commentedon Mar 30, 2015
What about specifying the origin in normalized axes coordinates? That would combine the flexibility of the
OffsetFromclass with the simplicity of just adding a kwarg toannotate.1 remaining item
tacaswell commentedon Jun 16, 2015
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 commentedon Jul 22, 2015
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:
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 commentedon Jul 22, 2015
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 commentedon Jul 22, 2015
Option 1 works for me.
myshen commentedon Jul 22, 2015
+1 option 1
+1 origin @tacaswell's comment is exactly the use case prompting my "fix".
tacaswell commentedon Jul 31, 2015
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
textcoordsto 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.Revert " Merge pull request matplotlib#4019 from myshen/annot_neg_coo…
efiring commentedon Aug 1, 2015