Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Propagate minpos from Collections to Axes.datalim#18642
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
this looks correct, but I'd need to spend more time with it to completely follow it. Can you add comments? Otherwise its pretty mysterious...
Uh oh!
There was an error while loading.Please reload this page.
| @@ -274,11 +274,11 @@ def get_datalim(self, transData): | |||
| # can properly have the axes limits set by their shape + | |||
| # offset. LineCollections that have no offsets can | |||
| # also use this algorithm (like streamplot). | |||
| result = mpath.get_path_collection_extents( | |||
| transform.get_affine(), paths, self.get_transforms(), | |||
| return mpath.get_path_collection_extents( | |||
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.
again, this could get some more explanation, given that there is a comment above, but this is clearly different. I'm not even sure I understand why you can change this like this.
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.
The first argument is the master transform; instead of doing one transform when finding extents, and a second one after on the result, this does a combined transform from the get-go. This ensures that whatever is calculated forminpos is in the final coordinate space.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This is already calculated by the internal C++ code, but discarded atthe end of the Python function.
This ensures that autoscaling on log scales is correct.
This test is a distilled out ofmatplotlib#16552.
This is mostly for the sake of third-party `Collection` subclasses thatmight have overridden `get_datalim`.
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.
Thanks!
PR Summary
This is an attempt tofix#16552. I'm not sure if it's the best change, API-wise, but seems to work without breaking anything. Possibly could do with an API change note.
Essentially, we have the right information to do log auto-scaling correct, but that's thrown away at the
Collection.get_datalim/Axes.add_collectioninterface. This propagates that information onto theBboxthat's passed between those two functions, and uses it when updatingAxes.dataLim.PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsandpydocstyle<4and runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).