FIX: Handle no-offsets in collection datalim (alternative) #22946
+43
−33
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds a check for whether the collection has offsets passed in initially. This is necessary to handle the (0, 0) offset case that is desired and distinguishing that from the initial empty offset of (0, 0). In particular, this is necessary for the non-transData IdentityTransform passed in during tight_layout calculations.
2 tasks
oscargus
reviewed
Apr 30, 2022
lib/matplotlib/collections.py
Outdated
| # default to zeros | ||
| self._offsets = np.zeros((1, 2)) | ||
|
|
||
| self._offsets = offsets |
There was a problem hiding this comment.
Could one move this to after the if-clause? (And more the identical line out of it.)
There was a problem hiding this comment.
Yep, good call, updated.
oscargus
approved these changes
Apr 30, 2022
This avoids carrying around an extra offsetsNone/has_offsets variable to keep track of the default case. Instead calling get_offsets() to return the default zeros case, and then internally checking the None/default case in get_datalim() which is the only place where this information is needed.
timhoffm
approved these changes
May 1, 2022
|
@meeseeksdev backport to v3.5.x |
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
greglucas
pushed a commit
to greglucas/matplotlib
that referenced
this pull request
May 1, 2022
…talim (alternative)
6 tasks
QuLogic
added a commit
that referenced
this pull request
May 2, 2022
…on-v3.5.x Backport PR #22946: FIX: Handle no-offsets in collection datalim
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Summary
Alternative to #22945
This keeps the None-default case in
self._offsetsand then transforms to the zeros for default math in theget_offsets()method. This is necessary to handle the (0, 0) offset case that is desired and distinguishing that from the initial empty offset of None. In particular, this is necessary for the non-transData IdentityTransform passed in during tight_layout calculations.closes #22921
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).