Undeprecate Cursor event handlers #25129
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit d2a3266.
So that the cursors show up on online docs
tacaswell
approved these changes
Feb 2, 2023
There was a problem hiding this comment.
I am convinced by the observation that as soon as the mouse is moved into the axes the cursor will be made visible again and already becomes invisible if the mouse moves out.
While there is a risk of breakage here, I think that it is less this way than with what is on main now.
tacaswell
reviewed
Feb 2, 2023
tacaswell
reviewed
Feb 2, 2023
d0af10f to
d8f7072
Compare
<
8000
/div>
Co-authored-by: Thomas A Caswell <tcaswell@gmail.com>
d8f7072 to
17542db
Compare
|
The current state works for me running |
anntzer
approved these changes
Feb 6, 2023
meeseeksmachine
pushed a commit
to meeseeksmachine/matplotlib
that referenced
this pull request
Feb 6, 2023
ksunden
pushed a commit
to meeseeksmachine/matplotlib
that referenced
this pull request
Feb 6, 2023
Trying to run the example prior to the PR would result in exceptions being thrown on mouse moves. Manual backport of matplotlib#25106 rolled into backport of matplotlib#25129 (backport pr matplotlib#25160)
ksunden
added a commit
that referenced
this pull request
Feb 7, 2023
…129-on-v3.7.x Backport PR #25129 on branch v3.7.x (Undeprecate Cursor event handlers)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #25107
PR Summary
The event handlers were deprecated for mpl 3.7 in #19763 (with correction for the deprecation notices in #24750)
This has not been released as a final matplotlib version (only the rc).
However, one of examples demonstrated subclassing these classes specifically to override these methods.
The callbacks registered internally were for the privitized version of the functions, thus the overrides did nothing.
Given that it is suggested in our docs, we determiened in #25107 that we should undeprecate these methods.
The original PR kept the calls to
set_visible(False)in the un-prefixed versions ofclearforCursorandMultiCursor.Because clear is registered as a callback on a
drawevent, this was causing the cursor to disappear on the nbagg backend.clearis not actually run in our test suite, as observed on codecov.In ad hoc testing, removing seemed to have limited impact on the qt backend (seemed to function as expected).
The problem from #19763 was reintroduced for the nbagg backend if the
set_visible(False)was included.So I removed those lines entirely for the undeprecation.
This feels odd, as I may have expected the
clearfunction to explicitly turn off the cursor visibility.But as far as I can tell, it works like I'd expect.
I suspect in the narrow case of a user calling
cleardirectly (as opposed to relying on the internal callbacks)it may not actually remove the lines and thus be unexpected behavior.
But I have no examples of doing such, and in most cases if the cursor was showing to begin with, the mouse is over the axes, and will cause the cursor to show up as soon as it moves anyway, I'd think.
Anywho, while working on these, figured I'd also add a simulted mouse event to the examples so that the docs show what they actually do (if not interactively still) a bit more.
I did so using
_process()on theMouseEventbecause that is what is done in our tests for simulating mouse events.I could have made the mouse event and passed it directly to the callback method, I think, which would have avoided using an
_method in our example.@anntzer suggested perhaps removing the widgets/annotated_cursor example all together because event_handlers/cursor_demo does effectively the same thing but in less code.
@tacaswell voted in favor of keeping both for the time being (though did check that they are cross linked to each other).
PR Checklist
Documentation and Tests
pytestpasses)Release Notes
.. versionadded::directive in the docstring and documented indoc/users/next_whats_new/.. versionchanged::directive in the docstring and documented indoc/api/next_api_changes/next_whats_new/README.rstornext_api_changes/README.rst