Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Factor out common parts of qt and macos interrupt handling. #27285

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Nov 7, 2023

Note that we don't actually need to disable the QSocketNotifier at the end, just letting it go out of scope should be sufficient as its destructor also does that (see qsocketnotifier.cpp in the qt source tree).

Followup to #27221.

(Also had to rename some local variables named "signal" in the CallbackRegistry implementation; otherwise the style checker complains about shadowing the imported module name... though #26013 would make some of these irrelevant.) Switched to putting the contextmanager in backend_bases.

PR summary

PR checklist

lib/matplotlib/cbook.py Outdated Show resolved Hide resolved
Note that we don't actually need to disable the QSocketNotifier at the
end, just letting it go out of scope should be sufficient as its
destructor also does that (see qsocketnotifier.cpp).
@anntzer
Copy link
Contributor Author

anntzer commented Nov 8, 2023

Thanks, fixed the copy editing. Also switched to putting the contextmanager in backend_bases, as that seems more specific than dumping it in cbook as a "general" utility.

Copy link
Contributor

@oscargus oscargus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyone can merge on green.

Good that you moved from cbook. Was about to suggest that.

@ksunden ksunden merged commit aeefb3b into matplotlib:main Nov 15, 2023
39 of 40 checks passed
@anntzer anntzer deleted the ai branch November 15, 2023 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants