-
Notifications
You must be signed in to change notification settings - Fork 62
Gridplot -> Figure
#479
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
Gridplot -> Figure
#479
Conversation
|
when reviewing don't be scared by the size of the diff, it's mostly stuff being moved around, and the changes to the examples are mostly renaming stuff from plot -> figure, gridplot -> figure 😄 |
clewis7
left a comment
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.
LGTM, mostly just a big refactor but I think this cleans up a lot of the soup and better organizes the repo
* fefactor imwidget, restrict to tzxy, txy, xy, added RGB(A) support (#459) * add new iw test screenshots * update api docs --------- Co-authored-by: Amol Pasarkar <amolpasarkar@gmail.com>
|
Idea: if edit: No, keep things consistent |
closes #457
This PR breaks everything 😄
Gridplotrenamed toFigurePlotis removed in favor of just doingFigure()since the defaultshape=(1, 1), i.e. "gridplot" with a single subplotFramemerged intoFigureRecordMixin->FigureRecorderrecordis now an instance ofFigure, use composition instead of inheritance, I haven't tested that this works yet, right now it's still a very experimental feature_framemodule renamed tooutput, some reorganization of what is public/private within that module to make it more logicalImageWidgetStill need to do this for
ImageWidget, waiting for #459 , rest of it is good to go and tests are passing on my end 🥳