search

enthought / chaco

Chaco is a Python package for building interactive and custom 2-D plots.
http://docs.enthought.com/chaco/
Other
292 stars 99 forks source link

Investigate use of both overlay and _draw_overlay in AbstractOverlays #767

Open aaronayres35 opened 3 years ago

aaronayres35 commented 3 years ago

For the full saga, see this comment thread: https://github.com/enthought/chaco/pull/743#discussion_r634611098

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: https://github.com/enthought/chaco/blob/5d692b382770c6ac97d8e4673875a54cc072ee59/chaco/abstract_overlay.py#L52-L62

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.