-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Fix: have apply_aspect keep track of user-set limits #30265
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
|
Did this not work? |
|
It worked, but as I said above I wasn’t sure about it because it seemed a bit hacky. Since nobody commented, I figured everyone else was also not sure about it. |
|
Possibly I could have labelled it differently to make it clearer I wanted feedback… |
|
Random thoughts - sorry if this somewhat incoherent; I don't have a full overview of what is going on here either I'm wondering whether we should strive for a more fundamental solution: The quantities Fundamentally we have these cases
Legend:
(*) The interesting part is in here. If data limits become variable, we must adapt one of xlim/ylim. And in theory one of them is enough. The bug is that we adjust both. My wild guess is that we keep one constant and adapt one as an induced effect to placate the above constraints. Then, constrained layout comes along and adjusts |
|
Thank you for your thoughts @timhoffm. I have not been ignoring you, but every time I try to think about this one I get quite lost! |
|
@rcomer No worries. Same here, as my disclaimer above reveals. 🫣 |
|
Thinking a bit more about it, I have now a more clear picture on the topic and a direction in which a solution must go. Side-note: I just realized that
Next steps: We need to clarify the cases and desired limit behavior for the IMHO there are three major cases to disinguish:
|
I think even for static plots this would not work well in general unless we can somehow make |
|
Checking the code, we currently may ignore explicitly set limits to ensure we are always increasing the range and not clipping data. matplotlib/lib/matplotlib/axes/_base.py Lines 2074 to 2089 in dedfe9b
While one could debate whether that's the best behavior, it at least is a clear and consistent solution; and since it's currently there, I'm ok with leaving it as is. This removes a lot of complication as the one-fixed case is logically the same as both-free (only that we issue a warning if we ignore the fixed limit). This means, we only have to solve the both-free case, in particular the change of limits/bounds through change of axes box size. And yes, I agree with the PR in that we have to track whether the bounds have been explicitly set or are an induced effect. We have to re-check how change of data limits interacts with this: What happens if I add new data points after the layout has done? Does the addition of new data result in bounds recalculation when a new layout is done? - It should. |
PR summary
Possibly closes #30230, though I'm unsure if it's a bit too hacky.
Introduce new private attributes so that
apply_aspectcan keep track of the previously set x/y-bounds. These attributes get reset whenset_xlim/set_ylimare called, so further user updates to the limits are respected.I ran the code from #30230 (comment) and messed about with the window shape. Everything seems fine.
If this seems good to others, I will add a test.
PR checklist