Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.7k
Allowshared_yaxes to work with secondary axes#5180
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
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
gmjw commentedMay 12, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This is the output of the test adjusted in the PR, with
|
This is my first PR to this repo, I've done my best but please let me know if there's anything I've forgotten or that could be done better. In particular I'm not sure if/how I should add to the CHANGELOG file. |
| [{"secondary_y": True}, {"secondary_y": True}], | ||
| ], | ||
| ) | ||
| for shared_y_axes in [False, True]: |
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.
Ideally I would have passed in these options usingmark.parametrize, but I don't think that is possible with unittest TestCase methods.
thanks@gmjw - I'll try to get this merged in for the 6.2 release. |
Thanks@gvwilson - just checking in to see if this could be merged soon - thanks! |
plotly/_subplots.py Outdated
| if secondary_y: | ||
| _configure_shared_axes( | ||
| layout, grid_ref, specs, "y", shared_yaxes, row_dir, True | ||
| ) |
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.
@gmjw I think there's a logic issue here --secondary_y is set in the loop above, which loops through all thespecs for each subplot. So after the above loop terminates,secondary_y corresponds only to the value provided forsecondary_y in the spec of the final subplot.
You can see how this causes issues if you set'secondary_y': False for the final subplot only; it breaks shared y-axes for every subplot in the grid.
The fix might be as simple as something like this:
| ifsecondary_y: | |
| _configure_shared_axes( | |
| layout,grid_ref,specs,"y",shared_yaxes,row_dir,True | |
| ) | |
| any_secondary_y=any([spec.get("secondary_y",False)forspec_rowinspecsforspecinspec_row]) | |
| ifany_secondary_y: | |
| _configure_shared_axes( | |
| layout,grid_ref,specs,"y",shared_yaxes,row_dir,True | |
| ) |
although I haven't fully thought through all the edge cases.
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.
Hi@emilykl - thanks for spotting this! I should have read through the function more carefully.
I've adjusted almost as you suggest - based on the loop above, it looks like I can rely on "secondary_y" being inspec providedspec is not None.
Uh oh!
There was an error while loading.Please reload this page.
The
shared_yaxesparameter to make_subplots did not previously work onsecondary y-axes. It had no effect.This PR fixes this, without affecting how the parameter is applied to normal plots (without secondary y-axes). I have added a simple test to show how the new code "matches" secondary y-axes. The sharing for secondary y-axes is applied right-to-left, in the same way that the sharing for primary y-axes is applied left-to-right (I believe this results in the clearest separation of tick marks when viewing the final plot).
I believe this is a bugfix/improvement. I'm not sure if it's a new feature worthy of any documentation.
I will add some images showing a before/after comparison in comments.