Skip to content

Updating MEP22 #8712

Closed
Closed
@anntzer

Description

@anntzer

@fariza @tacaswell

Although I like, in general, the idea of MEP22, there are a number of details that I would like to change. If we can agree on how to handle them, I would be happy to push the qt implementation. We'd still need a wx implementation (which would be painful for me to write but I guess I could cargo-cult it if no one else volunteers) and a macos implementation (which I would have no way to work on).

A quick look suggests the following points (I may think of other stuff later):

Suggested changes to MEP22

Change _get_cls_to_instantiate

Edit: Fixed by #20990.

Currently, backend_tools defines a default_tools dict, which maps tool names to either classes, or names of classes; for the latter, toolmanager.add_tool resolves the name in the current module context (e.g., the 'cursor': 'ToolSetCursor' tool resolves to whatever class is named ToolSetCursor in the current backend module). This looks brittle (e.g., one cannot override a tool that is specified directly as a class in default_tools). Instead, I propose that that default_tools simply becomes a list of default tools, that the name becomes a class attribute, and that overrides in backends get specified using a decorator

@tools.override(backend="qt5agg")
class ToolX(backend_tools.ToolX):
    ...

(possibly stacking decorators if the override applies to multiple backends). The decorator registers an override for the ancestor class(es), and would store the relevant override information somewhere for later use in add_tool.

Possibly, some default tools could be abstract, always requiring the backend to reimplement them, but not erroring if this is not the case (the tool is then just not available).

Get rid of the data kwarg to Tool.trigger

AFAICT this kwarg is only present so that ZoomTool can pass to RubberbandTool the coordinates where a rubberband needs to be drawn. I would simply get rid of RubberbandTool and let ZoomTool define the relevant drawing methods as abstract methods that need to be overriden by the individual backends.

Get rid of the sender kwarg to Tool.trigger?

(Its purpose is not clear to me, but perhaps I am missing something.)

Let togglable tools be implemented as contextmanagers (instead of enable/disable)

In general, this helps having to stick a bunch of temporary attributes on the instance to store the state. The caller code can always call __enter__ and __exit__ manually.

ToolBase.destroy(self, *args) should lose the *args in the signature

Edit: Fixed by #22509.

Why do we need it?

What is happening with NavigationBase?

The class does not seem to be implemented anywhere. A similar but not identical API seems implemented by ToolManager.

Support sub-tools as menus to the tool button?

Edit: Essentially fixed (from my POV) by figure.hooks.

e.g. #7696 (comment)

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions