| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
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. |
Sorry, something went wrong.
There was a problem hiding this comment.
Generally, 👍
matplotlib/lib/matplotlib/axes/_base.py
Line 2072 in 014aa5b
Sorry, something went wrong.
Places which used to call add_collection(..., autolim=False) should just set the collection's in_autoscale property to False before passing it to add_collection (and add_collection should be updated to take it that property into account).
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. |
Sorry, something went wrong.
There was a problem hiding this comment.
An example page would be nice..
Sorry, something went wrong.
|
will work on it |
Sorry, something went wrong.
|
(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)). |
Sorry, something went wrong.
|
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 scalex/scaley parameters in plt.plot. |
Sorry, something went wrong.
|
I see the point of being able to specify this per axis. Not completely thought this through API-wise, but a axis parameter similar to the one in tick_params might be the way to go. |
Sorry, something went wrong.
|
The input side of the API in clear and can be backwards ly, but what does the get side return? Do we want to do {'x': True, 'y': False}? |
Sorry, something went wrong.
|
Ymmv but I think an (x, y) pair would be fine |
Sorry, something went wrong.
|
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? |
Sorry, something went wrong.
|
How about using a namedtuple? |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
API proposal:Setterset_in_autoscale(False) # for both axises set_in_autoscale((False, True)) # for each axis s set_in_autoscale((False, True, False)) # for 3D 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 set_in_autoscale(*args) because you cannot have explicit arguments (set_in_autoscale(scalex, scaley)) and supporting a single value applying to all dimensions. But the single-parameter case is the primary usecase and excluding single dimensions is far more rare. Getterscalex, scaley = get_in_autoscale() scalex, scaley, scalez = get_in_autoscale() Per #15595 (comment) a simple tuple is good enough and saves the hassle of determining meaningful names, which can depend on the projection. Internal representationself._in_autoscale should always be a tuple with as many entries as dimensions in the plot (2D/3D). The expansion from scalar to tuple should be done in the setter. @robmarkcole Sorry this fell under the table. Are you still interested in working on this? |
Sorry, something went wrong.
|
Rebased to use this as the start of the implementation for #25139 |
Sorry, something went wrong.
|
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! |
Sorry, something went wrong.
|
This new property will be super useful! Could you explain the rationale for naming it in_autoscale instead of partly backward-compatible autolim from add_collections? I am implemeting autolim in geopandas and wondering if I should call it in_autoscale instead. Thanks! |
Sorry, something went wrong.
|
Any updates on this PR? would be a very useful feature. |
Sorry, something went wrong.
|
The following diagramm shows an outline of the limits-related call graph Currently the update is mostly done through the Axes methods add_[Artist] --> _update_[Artist]_limits --> update_datalim. Though there are quite a number of special cases:
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 Artist.set_in_autoscale into account. There are two ways: (1) Simple but expensive: set_in_autoscale() triggers a complete relim(). (2) Deferred limits update. Rearchitect futher so that data limits are calculated lazily. Basically autoscale would go through the artists and collect the limits. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
PR Summary
The Artist class gained an set_in_autoscale() method which
is used to determine if the instance is used in the autoscale calculation.
PR Checklist