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.

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

Investigate use of both overlay and _draw_overlay in AbstractOverlays #767

Open
aaronayres35 opened this issue Jun 8, 2021 · 0 comments

Comments

@aaronayres35
Copy link
Contributor

For the full saga, see this comment thread: #743 (comment)

The confusion comes from chaco defining its own AbstractOverlay which ends up calling the normal _draw method if its component is None. ie, chaco AbstractOverlays can be drawn standalone like any other component.
See:

def _draw(self, gc, view_bounds=None, mode="normal"):
"""Draws the component, paying attention to **draw_order**. If the
overlay has a non-null .component, then renders as an overlay;
otherwise, default to the standard PlotComponent behavior.
Overrides PlotComponent.
"""
if self.component is not None:
self.overlay(self.component, gc, view_bounds, mode)
else:
super()._draw(gc, view_bounds, mode)

Conversely in enable:
https://github.com/enthought/enable/blob/6e1c93e9df2a6e37eb79b92a81696ac757392dce/enable/abstract_overlay.py#L84-L90
that is not the case, so it would not be necessary to have a _draw_component method on the class (since there is no code path where it would be called)

The real question is, do all overlays in chaco need the ability to be drawn standalone? Some definitely do need to keep the _draw_overlay method as they do their own special things, eg Tooltip and its subclass DataLabel. However, it is unclear to me if PlotAxis or Grid actually need it. AFAICT, PlotAxis will always have its component set when used inside the chaco code base. I think its _draw_overlay method could be removed.

Note, this is overall probably low priority / the removals would only be to simplify code / make it more obvious what methods are for. Even if a method is unused, it isn't causing to much trouble. It may just be some added confusion for developers trying to distinguish which method does what / why both exist, etc.
More thorough documentation may be the simpler / less risky solution here.

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

No branches or pull requests

1 participant