-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add set_in_autoscale() method #15595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
PR suggested by @anntzer |
Note that this would not only be useful in itself, but would also fix a long-standing issue hinted at in matplotlib/lib/matplotlib/artist.py Lines 157 to 160 in 014aa5b
matplotlib/lib/matplotlib/axes/_base.py Lines 2065 to 2066 in 014aa5b
which basically asks for Collections to have such a flag -- and then we can get rid of the autolim kwarg in matplotlib/lib/matplotlib/axes/_base.py Line 1877 in 014aa5b
which is basically a one-off version of the property. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, 👍
- How does this relate to
autolim
parameter ofadd_collection()
? - This conceptually is similar to the visibility checks in
relim()
.
matplotlib/lib/matplotlib/axes/_base.py
Line 2072 in 014aa5b
if not visible_only or line.get_visible():
It would be good to have both checks either inside the_update_*_limits
or inrelim()
but not distributed over two places.
Places which used to call
That's a bit tricky to do, as relim() is a public API but we should not modify the state of the artist depending on whether visible_only was passed to relim. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An example page would be nice..
will work on it |
(Note to self: once this is merged this can also be used to control autolimits behavior in axline() as mentioned in the side note of #15330 (comment)). |
It was mentioned in #15967 that this method (which solves exactly a problem that I having been having) applies to both axes in a plot, but there may be times when you want to exclude autoscaling on one axis but allow it on another. In fact, there is some precedence for that independence, e.g. with the |
I see the point of being able to specify this per axis. Not completely thought this through API-wise, but a |
The input side of the API in clear and can be backwards ly, but what does the |
Ymmv but I think an (x, y) pair would be fine |
Blind tuples are pragmatic, but can lead to confusion if we start to lose track of what they mean. On the other hand, that ambiguity can be leverage to play nicely with polar plots. If we go with a tuple / dict / whatever we should also extend it to 3D as well? |
How about using a namedtuple? |
On a bit more consideration, I think that a tuple is probably the best path. The artists don't actually know if they are in an (x, y), (r, \theta), (ra, dec), or (lat, lon) coordinate system, they just know they are in 2D (or 3D) so trying to same them will end up causing more issues because it will be wrong rather than ambiguous. |
API proposal:Setter
Rationale: I find it more clear to have a single parameter that can be a scalar or a tuple compared to having variable number of arguments. In the latter case, we'd have to Getter
Per #15595 (comment) a simple tuple is good enough and saves the hassle of determining meaningful names, which can depend on the projection. Internal representation
@robmarkcole Sorry this fell under the table. Are you still interested in working on this? |
736eb57
to
c8adaa7
Compare
Rebased to use this as the start of the implementation for #25139 |
Tasks:
@robmarkcole Do you mind if we work in this PR / on your branch? I would rather do so because it keeps the discussion in one place, but also completely understand if you no longer are interested in getting notified about this discussion! |
This new property will be super useful! Could you explain the rationale for naming it |
Any updates on this PR? would be a very useful feature. |
The following diagramm shows an outline of the limits-related call graph Currently the update is mostly done through the
So overall every Artist is a special case 😲. I believe we should consolidate the architecture before modifying the autoscale behavior. We can do this in multiple steps:
To provide natural "in_autoscale" behavior, we'd need to take changes though |
PR Summary
The
Artist
class gained anset_in_autoscale()
method whichis used to determine if the instance is used in the autoscale calculation.
PR Checklist