Skip to content

Remove PanelAxes class, remove panel references from "subplots()" #31

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Sep 5, 2019

Conversation

lukelbd
Copy link
Collaborator

@lukelbd lukelbd commented Sep 5, 2019

This is the major API change discussed in this thread. There is no longer a PanelAxes class, and now "panel" colorbars and legends can only be implemented on-the-fly. The fact that colorbars and legends lying outside of axes are generated by "filling panels" is now entirely hidden from the user. Users can still generate panels for plotting stuff using the panel and panel_axes commands.

I've also made a major improvement that makes it easier to "stack" panels. Previously, stacked panels were embedded in FlexibleGridSpecFromSubplotSpec objects. This was problematic, because it made it difficult to align axes panels in adjacent rows and columns, and it meant the user had to specify the number of panels in the "stack" a priori during the plot.subplots() call. Now, panels are "stacked" when the user calls colorbar, legend, or panel_axes more than once on the same side.

To permit only on-the-fly "panel" colorbars and legends, but preserve the "stacked panels" feature, ProPlot now inserts both axes and figure panels directly intoFigure._gridspec_main grid spec. When a "panel" row or column has not yet been allocated, a new gridspec is generated, and the subplotspec is updated for all axes in the figure. This method is also much cleaner. Previously, I was populating the gridspec with "empty", zero-width rows and columns allocated for panels during the subplots() call. This was a headache for many reasons, especially when trying to do the "tight layout" calculations.

Finally, I've updated the documentation to reflect these changes.

In the future I'll try to be better about my commit messages -- this is my first pull request.

@lukelbd lukelbd merged commit 4a43155 into master Sep 5, 2019
@bradyrx
Copy link
Collaborator

bradyrx commented Sep 5, 2019

Yay! PR with merge commit makes the commits way less polluted so it's easier to track updates to the package, revert to earlier commit hashes, etc, and update the CHANGELOG directly with a pointer to a PR. Good stuff

@lukelbd lukelbd deleted the panels-change-gridspec branch September 9, 2019 20:54
@lukelbd
Copy link
Collaborator Author

lukelbd commented Sep 17, 2019

Btw at this point I can't believe I was working so long without branches.... even without automatic PR testing they are super useful just for organizational purposes/clarity in working through 5 different trains of thought/isolating bugs that are potentially introduced. Now I'm trying to only commit to master for documentation changes, very small-scale API enhancements, and minor bugfixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants