Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

don't override non-Python signal handlers#16311

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
tacaswell merged 2 commits intomatplotlib:masterfromstevengj:patch-1
Jan 25, 2020

Conversation

stevengj
Copy link
Contributor

@stevengjstevengj commentedJan 23, 2020•
edited
Loading

The qt5 backend'smainloop() function (used by the non-interactiveshow()) callssignal(SIGINT, SIG_DFL) to temporarily override theSIGINT handler while the plot window is open. Unfortunately, this fails when Python has been embedded in another program and a non-Python signal handler is installed, as we encountered in Julia atJuliaPy/PyPlot.jl#459.

In particular, whensignal.getsignal(SIGINT) returnsNone, this isdocumented to mean that "the previous signal handler was not installed from Python." In this case, you should not override the signal handler because there is no way to restore it — thesignal(SIGINT, old_signal) function throws aTypeError ifold_signal isNone.

The fix is simple: don't try to override theSIGINT handler ifold_signal isNone.

Copy link
Contributor

@anntzeranntzer left a comment

Choose a reason for hiding this comment

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

conditional on style fix and ci passing.

@tacaswelltacaswell modified the milestones:v3.1.3,v2.2.5Jan 25, 2020
@tacaswelltacaswell merged commitd162b6d intomatplotlib:masterJan 25, 2020
@lumberbot-app
Copy link

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
$ git checkout v2.2.x$ git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
$ git cherry-pick -m1 d162b6d2b8d001f893bc7a0a073f77f3ff445847
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
$ git commit -am "Backport PR #16311: don't override non-Python signal handlers"
  1. Push to a named branch :
git push YOURFORK v2.2.x:auto-backport-of-pr-16311-on-v2.2.x
  1. Create a PR against branch v2.2.x, I would have named this PR:

"Backport PR#16311 on branch v2.2.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free tosuggest an improvement.

@tacaswell
Copy link
Member

The 2.2.x branch does not have the restoring logic, rather than implicitly backport that as well, only push this back to 3.1.x

@tacaswell
Copy link
Member

Thanks@stevengj ! Congratulations on your first merged Matplotlib pull request 🎉 Hopefully we will hear from you again.

stevengj reacted with heart emoji

timhoffm added a commit that referenced this pull requestJan 25, 2020
…311-on-v3.1.xBackport PR#16311 on branch v3.1.x (don't override non-Python signal handlers)
timhoffm added a commit that referenced this pull requestJan 25, 2020
…311-on-v3.2.xBackport PR#16311 on branch v3.2.x (don't override non-Python signal handlers)
@stevengjstevengj deleted the patch-1 branchJanuary 25, 2020 14:13
@timjim333
Copy link

timjim333 commentedJan 25, 2020•
edited
Loading

Hi, I had a similar issue that only appeared when I ran Matplolib fromParaview's pvpython - is this fix something that I can implement or do I have to wait for the next release on conda? Thanks and regards. Tim

Edit: I just applied the 2-line fix detailed in the link at the top of the page manually in my conda-managed file:
~/anaconda3/envs/sound6/lib/python3.7/site-packages/matplotlib/backends/backend_qt5.py

I'm not sure if this is the best idea or not, but it fixes the issue for the moment!

stevengj reacted with hooray emoji

@timjim333
Copy link

Sorry, just to add an additional comment (I know you've already sent off the patch) - would it be a little more robust to checkif old_signal is not None? I realise that in this case, the non-None returns ofsignal.getsignal(SIGINT) (signal.SIG_IGN andsignal.SIG_DFL) both currently evaluate to Python 'truthy', so that justif old_signal works at the moment, but would this in some foreseeable circumstance change? Not a criticism but a question, as I'm not well versed in these libraries! Thanks for the fixes!

@stevengj
Copy link
ContributorAuthor

From my understanding of the Python docs, any valid signal handler will have a True truth value.is not None would be fine too, of course.

timjim333 reacted with thumbs up emoji

@timhoffmtimhoffm mentioned this pull requestJan 26, 2020
tacaswell added a commit to vdrhtc/matplotlib that referenced this pull requestAug 12, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@anntzeranntzeranntzer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.1.3
Development

Successfully merging this pull request may close these issues.

4 participants
@stevengj@tacaswell@timjim333@anntzer
[8]ページ先頭
©2009-2025 Movatter.jp