Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Handle zorder between overlayed cartesian subplots #7032

Merged
merged 7 commits into from
Jun 25, 2024
Merged

Handle zorder between overlayed cartesian subplots #7032

merged 7 commits into from
Jun 25, 2024

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Jun 19, 2024 •
edited

Supersedes #6943.
Fixes #6943.

Changes to make zorder work between overlayed cartesian subplots!

@archmoj archmoj added bug something broken feature something new status: has TODOs labels Jun 19, 2024
@gvwilson gvwilson self-requested a review June 21, 2024 11:10
@archmoj
handle zorder between overlayed cartesian subplots
8ed564c 
 - by adding zindex layers e.g. xyz2, x2yz2, xy2z2, x2y2z2, etc.
 - move zorder logic from plotOne subplot to plot all subplots
 - add test
@archmoj archmoj requested review from emilykl and Farkites June 24, 2024 16:37
Farkites
Farkites approved these changes Jun 25, 2024
View reviewed changes
Copy link
Contributor

@Farkites Farkites left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works like a charm!!! I have tested the sample code provided by the sponsors and it behaves as expected. Other cases also work correctly. Good job!! 💃 Thanks @archmoj!

@gvwilson
Copy link
Contributor

woo hoo!

@archmoj archmoj merged commit e365e8e into master Jun 25, 2024
1 check passed
@archmoj archmoj deleted the zorder-overlays branch June 25, 2024 13:24
@emilykl
Copy link
Contributor

emilykl commented Jun 26, 2024

Awesome PR @archmoj 🏆 🌟

Am I understanding the code correctly, that the approach here is simply to create separate subplots for each individual zorder value, and plot them from lowest to highest? Makes sense to me.

@archmoj
Copy link
Contributor Author

archmoj commented Jul 2, 2024

Awesome PR @archmoj 🏆 🌟

Am I understanding the code correctly, that the approach here is simply to create separate subplots for each individual zorder value, and plot them from lowest to highest? Makes sense to me.

@emilykl yes that's correct.
I'm wondering now that with this in place is it possible to simplify some of the logic added in #6918?

@archmoj archmoj added this to the v2.34.0 milestone Jul 4, 2024
@archmoj archmoj mentioned this pull request Jul 23, 2024
Closed
@FlorentinTh FlorentinTh mentioned this pull request Aug 9, 2024
Open
@guaiya0507 guaiya0507 mentioned this pull request Aug 9, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Farkites Farkites Farkites approved these changes

@gvwilson gvwilson Awaiting requested review from gvwilson

@emilykl emilykl Awaiting requested review from emilykl

Assignees
No one assigned
Labels
bug something broken feature something new
Projects
None yet
Milestone
v2.34.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@archmoj @gvwilson @emilykl @Farkites