enthought / enthought/traitsui
Awkward editor disposal when editors are defined by subclassing another
- Dominant language
- Python
- Stars
- 306
- Forks
- 99
- PR merge metrics
- No merged PRs in 30d
Description
This pattern is seen in a number of editors: a concrete editor is defined by subclassing another concrete editor. By concrete editors, I mean a subclass of `traitsui.editor.Editor` that is intended to be instantiated directly to be used in `traitsui.ui.UI._editors`.
For example, there is a concrete editor `traitsui.qt4.text_editor.SimpleEditor`, which is subclassed by another concrete editor `traitsui.qt4.text_editor.CustomEditor`.
This makes implementing the `dispose` method awkward when trying to clean up widgets upon UI disposal.
e.g.
`traitsui.qt4.button_editor.CustomEditor` subclasses `traitsui.qt4.button_editor.SimpleEditor` in order to reuse the `update_object` and `update_editor` defined there.
`SimpleEditor` has `init` and `dispose` methods. However `CustomEditor` only overrides `SimpleEditor`'s `init`, and does not implement a `dispose`.
The `SimpleEditor.dispose` is in fact incorrect: It needs to remove the on_trait_change handlers put up in `SimpleEditor.init`. However, such removal is NOT needed by `CustomEditor`. Consequently, one may need to override `dispose` in `CustomEditor`, but then it will also need to call `traitsui.editor.Editor.dispose`. Simply calling `super().dispose()` will end up calling `SimpleEditor.dispose` which will try to remove change handlers that don't exist.
Proposed solutions (they are not mutually exclusive):
- Avoid subclassing editors that are intended to be a leaf class: Refactor the common logic to a base class that is subclassed by the concrete editors.
- Move the logic in `traitsui.editor.Editor.dispose` out to another maybe-private method so that it is always run without relying on subclass to always call it. Its docstring says it is intended to be overridden to provide *additional clean-up*. If the base class implementations always needs to be run, it might make sense for the base class to ensure that happens.
- More tools for disposal: Maybe we need a method for adding cleanup steps in a stack, similar to `unittest.TestCase.addCleanup`. `unittest.TestCase.setUp` will be analogous to `traitsui.editor.Editor.init`. `unittest.TestCase.tearDown` is analogous to `traitsui.editor.Editor.dispose`.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.