Move set_cursor from the toolbar to FigureCanvas. - #20620
Conversation
QuLogic
added
the
API: changes
There was a problem hiding this comment.
Side note:
We should move away from cursors and switch to directly using the enum Cursors everywhere. Possbibly also deprecate cursors, Not sure if we can already issue warnings on module variables. If not, still announce the deprecation, and extend the duration by 1 or 2 extra releases. When people have to touch set_cursor anyway, they should switch to the enum while they are at it. This need not (possibly should not) be part of this PR for simplicity, but it should be a follow-up
| _api.warn_deprecated("3.5", | ||
| message="Overriding ToolSetCursor with " | ||
| f"{tool_cls.__qualname__} was only " | ||
| "necessary to provide the .set_cursor() " | ||
| "method, which is deprecated since " | ||
| "%(since)s and will be removed " | ||
| "%(removal)s") | ||
|
|
There was a problem hiding this comment.
I don't quite follow this. From a user perspective, under which conditions is the warning emitted, and what action should they take if they see this message?
There was a problem hiding this comment.
None, in Matplotlib. It's for third-party backends, of which I'm not sure any even implement tool manager. I'll tweak the message to say so.
There's module-level |
|
I think #10735 is what you are looking for :-) (or some variants thereof, I can open a new PR if there's interest) |
timhoffm
left a comment
timhoffm
left a comment
There was a problem hiding this comment.
Modulo one comment on a deprecation.
While the toolbar does need to track what it wants to set the cursor to based on the active tool, actually setting it is an act on the canvas (or sometimes the window, but this is toolkit dependent, so we won't worry about that.)
Which is now replaced by `FigureCanvasBase.set_cursor`.
4 participants
PR Summary
While the toolbar does need to track what it wants to set the cursor to based on the active tool, actually setting it is an act on the canvas (or sometimes the window, but this is toolkit dependent, so we won't worry about that.)
It also means we don't need redundant
set_cursordefinitions on both toolbar/toolmanager(not yet committed.)PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).