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

Use stack layout helpers from enable #757

Merged
merged 3 commits into from
Jun 7, 2021
Merged

Use stack layout helpers from enable #757

merged 3 commits into from
Jun 7, 2021

Conversation

rahulporuri
Copy link
Contributor

@rahulporuri rahulporuri commented Jun 7, 2021 •
edited
Loading

There was duplicate code sitting in chaco that handled getting the preferred stacked container size and that handled doing the layout. This PR updates chaco to make use of the existing helper functions in enable. This work has been extracted from #674 .

Ref https://github.com/enthought/enable/blob/6e1c93e9df2a6e37eb79b92a81696ac757392dce/enable/stacked_layout.py

Note that we would like this PR to be backported to the maint/5.0 branch so that it can be included in the chaco 5.0 release.

CLN : Use stack layout helpers from enable
40eefff 
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
@rahulporuri rahulporuri mentioned this pull request Jun 7, 2021
Closed
aaronayres35
aaronayres35 approved these changes Jun 7, 2021
View reviewed changes
Copy link
Contributor

@aaronayres35 aaronayres35 left a comment

Choose a reason for hiding this comment

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

LGTM

I will try to keep the need for these helpers in chaco in mind when reviewing the upcoming refactoring steps

@rahulporuri
Copy link
Contributor Author

I will try to keep the need for these helpers in chaco in mind when reviewing the upcoming refactoring steps

In the next refactoring step, we will actually be able to get rid of these helpers entirely from chaco - because we'll directly inherit from the enable base classes which use the helpers internally.

This PR reduced the eventual diff, which i hoped would make reviews easier. See #674 for what the h/v stacked containers will eventually look like - more or less.

@rahulporuri rahulporuri merged commit 01cb3f3 into master Jun 7, 2021
@rahulporuri rahulporuri deleted the cln/use-enable-layout-helpers branch June 7, 2021 15:09
rahulporuri pushed a commit that referenced this pull request Jun 8, 2021
Use stack layout helpers from enable (#757)
536475e 
* 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
@rahulporuri rahulporuri mentioned this pull request Jun 8, 2021
Merged
rahulporuri pushed a commit that referenced this pull request Jun 8, 2021
Backport Plot Containers refactor to 5.0 maintenance branch (#765)
a7e0a6d 
* 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
Reviewers

@aaronayres35 aaronayres35 aaronayres35 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rahulporuri @aaronayres35