NotificationsYou must be signed in to change notification settings
Fork750
Star5.1k

Rebased PR 450 (Py_Initialize without overriding signals)#724

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

Jump to bottom

Closed

filmor wants to merge7 commits intopythonnet:masterfromfilmor:449_Py_Initialize_SignalConfiguration

Closed

Rebased PR 450 (Py_Initialize without overriding signals)#724

filmor wants to merge7 commits intopythonnet:masterfromfilmor:449_Py_Initialize_SignalConfiguration

Conversation

Copy link

Member

What does this implement/fix? Explain your changes.

Avoiding overriding signals from the application so that CTRL-C might be handled.

Using Py_InitializeEx(0) (instead Py_Initialize) to avoid overriding signals.

Does this close any currently open issues?

#449 #450

Any other comments?

Just a rebased version of#450.

Checklist

Check all those that are applicable and complete.

Make sure to include one or more tests for your change
If an enhancement PR, please create docs and at best an example
Add yourself toAUTHORS
Updated theCHANGELOG

filmor requested review fromden-run-ai anddmitriyse

August 21, 2018 14:28

Copy link

Contributor

den-run-ai commentedAug 22, 2018

@filmor why is this failing CI?

Copy link

MemberAuthor

filmor commentedAug 22, 2018

Something aboutCSharpCodeProvider, I'll look into it. It's the same error that@rmadsen-ks had already when he pushed the PR, I think.

filmor mentioned this pull request

Aug 30, 2018

WIP: 449 Py_Initialize_SignalConfiguration#450

Closed

4 tasks

filmor changed the title~~Rebased PR 450~~Rebased PR 450 (Py_Initialize without overriding signals)

Aug 30, 2018

filmor added this to the2.4.0 milestone

Aug 30, 2018

Copy link

codecovbot commentedOct 16, 2018•
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@e0f47ba).Click here to learn what that means.
The diff coverage is66.66%.

@@            Coverage Diff            @@##             master     #724   +/-   ##=========================================  Coverage          ?   75.58%           =========================================  Files             ?       60             Lines             ?     5627             Branches          ?      888           =========================================  Hits              ?     4253             Misses            ?     1060             Partials          ?      314

Flag	Coverage Δ
#setup_linux	`68.55% <ø> (?)`
#setup_windows	`74.56% <66.66%> (?)`

Impacted Files	Coverage Δ
src/runtime/runtime.cs	`90.9% <0%> (ø)`
src/runtime/pythonengine.cs	`78.09% <100%> (ø)`

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last updatee0f47ba...3d6024d. Read thecomment docs.

filmor force-pushed the449_Py_Initialize_SignalConfiguration branch from168ec06 toe2b8c25Compare

October 29, 2018 12:47

filmor added the next label

Oct 29, 2018

Copy link

Contributor

rmadsen-ks commentedNov 13, 2018

@filmor, the only change in this code is optionally calling InitializeEx instead of Initialize. This code is often being touched by tests. The actual change here is really simple, the unit test is excessively complex. Could you consider merging this without a unittest testing CTRL+C explicitly?