Add set_offset3d and get_offset3d methods to 3D Collections#27556
Closed
scottshambaugh wants to merge 9 commits intomatplotlib:mainfrom
Closed
Add set_offset3d and get_offset3d methods to 3D Collections#27556scottshambaugh wants to merge 9 commits intomatplotlib:mainfrom
set_offset3d and get_offset3d methods to 3D Collections#27556scottshambaugh wants to merge 9 commits intomatplotlib:mainfrom
Conversation
bbd244b to
a6c2fac
Compare
Contributor
Author
|
Only test failure is codecov, but these are fairly well covered and should be fine. |
timhoffm
reviewed
Mar 10, 2024
Member
timhoffm
left a comment
There was a problem hiding this comment.
Minor style fixes / improvements.
timhoffm
reviewed
Mar 10, 2024
Member
|
I took the liberty to create a commit with the style fixes. |
8f94462 to
d2b39dd
Compare
timhoffm
reviewed
Mar 10, 2024
070dfab to
54bd200
Compare
Member
|
Could you add a simple test? I think just manipulating the values and reading them back to assert that you get the new offesets is sufficient. Or, if you want to be a bit more fancy make an image comparison test where you create a collection in |
Contributor
Author
|
Moving back to draft since I need to dig more into what Poly3DCollection is doing |
e2b6454 to
fc034a7
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Addresses #784 in part
More discussion in #19573
Note that the inherited
Collection.set_offsetandCollection.get_offsetmethods with 2D coordinates are used in zordering calculations and I couldn't see an easy way to abstract those out in order to override those methods and keep the same names as the 2D case.PR checklist