Make chaco v/h plot containers inherit from enable v/h stacked containers #764
Conversation
This file contains 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
added 10 commits
June 7, 2021 20:45
specifically, HPlotContainer and VPlotContainer subclasses. this is a small step towards removing the inheritance from StackedPlotContainer modified: chaco/plot_containers.py
specifically, we redefine get_preferred_size and _do_stack_layout methods on the subclasses of StackedPlotContainer modified: chaco/plot_containers.py
with these two traits, we have finally duplicated all methods and traits defined on the StackedPlotContainer in its subclasses. In the next step, we can finally remove the inheritance from StackedPlotContainer modified: chaco/plot_containers.py
and make the subclasses inherit directly from BasePlotContainer instead. also remove the StackedPlotContainer class. We don't need to worry about external users of StackedPlotContainer as the class is private to this module and isn't exposed outside of this module. See __all__ in chaco.plot_containers modified: chaco/plot_containers.py
and with this, we can remove their dependence on BasePlotContainer as well modified: chaco/plot_containers.py
we can now start the process of using stacked container classes in enable modified: chaco/plot_containers.py
and remove a number of now redundant trait definitions and methods modified: chaco/plot_containers.py
and remove a number of now redundant trait and methods definitions. also remove unused imports from enable. modified: chaco/plot_containers.py
_cached_preferred_size is expected and set by the layout machinery. it is defined explicitly on the HPlotContainer and OverlayPlotContainer but not on the VPlotContainer earlier modified: chaco/plot_containers.py
the classes were added to enable.api on the main branch - and they arent available in a released version yet modified: chaco/plot_containers.py
This comment has been minimized.
This comment has been minimized.
instead of installing enable 5.1.1 via edm this is a temporary solution until enable 5.2.0 is available via edm - at which point the changes in ci should be removed modified: ci/edmtool.py
because CI now needs to build enable from source modified: .github/workflows/test-with-edm.yml
aaronayres35
approved these changes
Jun 7, 2021
There was a problem hiding this comment.
LGTM, thank you very much for the commit breakdown! It made the changes far more tractable
Is the plan then to hold off on merging until enable 5.2 is available on edm, and then revert the edmtool / gh actions changes before pushing this through? We can also update enable to use transient metadata effectively / pull up the __getstate__ prior to 5.2.0 release and remove the __getstate__ methods in this PR before it is merged.
Edit: I've opened enthought/enable#841
chaco/plot_containers.py
Outdated
| @@ -129,51 +124,8 @@ class HPlotContainer(Container): | |||
|
|
|||
| draw_order = Instance(list, args=(DEFAULT_DRAWING_ORDER,)) | |||
|
|
|||
| # The dimension along which to stack components that are added to | |||
| # this container. | |||
| stack_dimension = Str("h", transient=True) | |||
There was a problem hiding this comment.
we will want to set transient=True in enable.stacked_container.StackedContainer
I'm actually hoping that we can merge this PR as-is - and eventually update ci to use the enable 5.2.0 release tag - and after the chaco release is also complete, revert everything to use edm eggs instead of installing from git source. I don't see any other way by which we can make a chaco rc2 that includes these refactor PRs.
Yup. These would be nice and thanks for opening the PR. They can be removed after the PR is merged. I don't see a reason why they should block the release. Are you okay with me merging this PR as-is and cherry-picking the commits back into the 5.2.0 maintenance branch? |
rahulporuri
pushed a commit
that referenced
this pull request
Jun 8, 2021
…ners (#764) * REF : Redefine class-level attr on StackedPlotContainer subclasses specifically, HPlotContainer and VPlotContainer subclasses. this is a small step towards removing the inheritance from StackedPlotContainer modified: chaco/plot_containers.py * REF : Redefine methods defined on StackedPlotContainer specifically, we redefine get_preferred_size and _do_stack_layout methods on the subclasses of StackedPlotContainer modified: chaco/plot_containers.py * REF : Redefine stack_dimension and other_dimension on subclasses with these two traits, we have finally duplicated all methods and traits defined on the StackedPlotContainer in its subclasses. In the next step, we can finally remove the inheritance from StackedPlotContainer modified: chaco/plot_containers.py * CLN : Remove inheritance from StackedPlotContainer and make the subclasses inherit directly from BasePlotContainer instead. also remove the StackedPlotContainer class. We don't need to worry about external users of StackedPlotContainer as the class is private to this module and isn't exposed outside of this module. See __all__ in chaco.plot_containers modified: chaco/plot_containers.py * REF : Redefine BasePlotContainer traits on subclasses and with this, we can remove their dependence on BasePlotContainer as well modified: chaco/plot_containers.py * REF : chaco stacked plot containers now inherit from enable Container we can now start the process of using stacked container classes in enable modified: chaco/plot_containers.py * CLN : Make HPlotContainer inherit from enable HStackedContainer and remove a number of now redundant trait definitions and methods modified: chaco/plot_containers.py * REF : Make VPlotContainer inherit from enable VStackedContainer and remove a number of now redundant trait and methods definitions. also remove unused imports from enable. modified: chaco/plot_containers.py * FIX : Define necessary private cache trait on VPlotContainer _cached_preferred_size is expected and set by the layout machinery. it is defined explicitly on the HPlotContainer and OverlayPlotContainer but not on the VPlotContainer earlier modified: chaco/plot_containers.py * FIX : Dont import stacked containers from enable.api the classes were added to enable.api on the main branch - and they arent available in a released version yet modified: chaco/plot_containers.py * FIX : Install enable from maint/5.2 branch from git source instead of installing enable 5.1.1 via edm this is a temporary solution until enable 5.2.0 is available via edm - at which point the changes in ci should be removed modified: ci/edmtool.py * FIX : Install GL dependencies even on null toolkit because CI now needs to build enable from source modified: .github/workflows/test-with-edm.yml
rahulporuri
pushed a commit
that referenced
this pull request
Jun 8, 2021
* Refactor chaco OverlayPlotContainer to inherit from enable OverlayContainer (#758) * REF : Redefine container_under_layers in OverlayPlotContainer the trait is already defined in BasePlotContainer but we redefine it in the subclass so that we can eventually stop inheriting from it modified: chaco/plot_containers.py * REF : Redefine draw_layer trait in subclass like before, this trait is already defined in BasePlotContainer but we duplicate it in the subclass so that we can stop inheriting from BasePlotContainer modified: chaco/plot_containers.py * REF : Stop inheriting from BasePlotContainer all of the traits defined in BasePlotContainer have been redefined in OverlayPlotContainer. so OverlayPlotContainer can just inherit from Container directly modified: chaco/plot_containers.py * REF : Make OverlayPlotContainer inherit from enable OverlayContainer and remove the now redundant definitions of the methods get_preferred_size and _do_layout modified: chaco/plot_containers.py * Use stack layout helpers from enable (#757) * CLN : Use stack layout helpers from enable there was duplicate code sitting in chaco that handled getting the preferred stacked container size and that handled doing the layout. this commit updates chaco to make use of the existing helper functions in enable modified: chaco/plot_containers.py * Fix bad merge * Make chaco v/h plot containers inherit from enable v/h stacked containers (#764) * REF : Redefine class-level attr on StackedPlotContainer subclasses specifically, HPlotContainer and VPlotContainer subclasses. this is a small step towards removing the inheritance from StackedPlotContainer modified: chaco/plot_containers.py * REF : Redefine methods defined on StackedPlotContainer specifically, we redefine get_preferred_size and _do_stack_layout methods on the subclasses of StackedPlotContainer modified: chaco/plot_containers.py * REF : Redefine stack_dimension and other_dimension on subclasses with these two traits, we have finally duplicated all methods and traits defined on the StackedPlotContainer in its subclasses. In the next step, we can finally remove the inheritance from StackedPlotContainer modified: chaco/plot_containers.py * CLN : Remove inheritance from StackedPlotContainer and make the subclasses inherit directly from BasePlotContainer instead. also remove the StackedPlotContainer class. We don't need to worry about external users of StackedPlotContainer as the class is private to this module and isn't exposed outside of this module. See __all__ in chaco.plot_containers modified: chaco/plot_containers.py * REF : Redefine BasePlotContainer traits on subclasses and with this, we can remove their dependence on BasePlotContainer as well modified: chaco/plot_containers.py * REF : chaco stacked plot containers now inherit from enable Container we can now start the process of using stacked container classes in enable modified: chaco/plot_containers.py * CLN : Make HPlotContainer inherit from enable HStackedContainer and remove a number of now redundant trait definitions and methods modified: chaco/plot_containers.py * REF : Make VPlotContainer inherit from enable VStackedContainer and remove a number of now redundant trait and methods definitions. also remove unused imports from enable. modified: chaco/plot_containers.py * FIX : Define necessary private cache trait on VPlotContainer _cached_preferred_size is expected and set by the layout machinery. it is defined explicitly on the HPlotContainer and OverlayPlotContainer but not on the VPlotContainer earlier modified: chaco/plot_containers.py * FIX : Dont import stacked containers from enable.api the classes were added to enable.api on the main branch - and they arent available in a released version yet modified: chaco/plot_containers.py * FIX : Install enable from maint/5.2 branch from git source instead of installing enable 5.1.1 via edm this is a temporary solution until enable 5.2.0 is available via edm - at which point the changes in ci should be removed modified: ci/edmtool.py * FIX : Install GL dependencies even on null toolkit because CI now needs to build enable from source modified: .github/workflows/test-with-edm.yml
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR refactors the
HPlotContainerandVPlotContainerclasses in enable to inherit from the enableHStackedContainerandVStackedContainerrespectively. This removes duplicate code between chaco and enable. In the end, in our opinion, the*PlotContainerclasses in chaco are more-readable than they were before.Note that this PR was written with enable main branch in mind - but it should work with the current release of
enableas well. This PR is what will introduce the dependency onenable >= 5.2on the upcomingchacomajor release.This PR can be reviewed one commit at a time as we have taken great care to keep the commits self-contained. Each commit makes only one type of change, which we hope will make the review process simpler.
This is the last big PR extracted from #674.
Note that we would like this PR to be backported to the
maint/5.0branch so that it can be included in the chaco 5.0.0 major release.