René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Updating MEP22","articleBody":"@fariza @tacaswell \r\n\r\nAlthough 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).\r\n\r\nA quick look suggests the following points (I may think of other stuff later):\r\n\r\nSuggested changes to MEP22\r\n\r\n### Change `_get_cls_to_instantiate`\r\n\r\nEdit: Fixed by #20990.\r\n\r\nCurrently, `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\r\n```\r\n@tools.override(backend=\"qt5agg\")\r\nclass ToolX(backend_tools.ToolX):\r\n    ...\r\n```\r\n(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`.\r\n\r\nPossibly, 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).\r\n\r\n### Get rid of the `data` kwarg to `Tool.trigger`\r\n\r\nAFAICT 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.\r\n\r\n### Get rid of the `sender` kwarg to `Tool.trigger`?\r\n\r\n(Its purpose is not clear to me, but perhaps I am missing something.)\r\n\r\n### Let togglable tools be implemented as contextmanagers (instead of enable/disable)\r\n\r\nIn 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.\r\n\r\n### `ToolBase.destroy(self, *args)` should lose the `*args` in the signature\r\n\r\nEdit: Fixed by #22509.\r\n\r\nWhy do we need it?\r\n\r\n### What is happening with NavigationBase?\r\n\r\nThe class does not seem to be implemented anywhere.  A similar but not identical API seems implemented by ToolManager.\r\n\r\n### Support sub-tools as menus to the tool button?\r\n\r\nEdit: Essentially fixed (from my POV) by figure.hooks.\r\n\r\ne.g. https://github.com/matplotlib/matplotlib/pull/7696#issuecomment-306069093","author":{"url":"https://github.com/anntzer","@type":"Person","name":"anntzer"},"datePublished":"2017-06-04T00:17:52.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":45},"url":"https://github.com/8712/matplotlib/issues/8712"}