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

ENH skip NPY_ALLOW_C_API for UFUNC_ERR_PRINT #6077

Merged
merged 1 commit into from
Jul 23, 2015
Merged

Conversation

ndjensen
Copy link
Contributor

GIL unnecessary when numpy floating point error handling is set to print
potential micro-optimization when error handling is set to print
alleviates (but does not fix) #5856 deadlock with sub-interpreters

Change-Id: I0414df8c5dca131e1f8c8d867791ba63cf992b63

@seberg
Copy link
Member

seberg commented Jul 19, 2015

Frankly, I am have not much clue about this, but the tests error out because you have to put the NPY_ALLOW_C_API_DEF before the statement (C89 compatibility -- -Werror=declaration-after-statement). While at it, can you could also remove the ; from it (it is prettier to put it, but it is already included in the macro, and ;; is bad because then you cannot add more defs after this defs for the same compatibility reason).
Anyway, generally seems right to me, not too pretty, but oh well....


NPY_ALLOW_C_API_DEF

// don't need C API for a simple print
Copy link
Member

Choose a reason for hiding this comment

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

I am sorry, I seem to have forgotten to mention that this also should use /* blah */ style comments, // are C++ style comments.
After that, please squash the commits, and I think we can merge it (might want to post a message, otherwise nobody notices the update), if you can think of a way to add a test, that would be great, but I am not sure there is anything not overly complex.

GIL unnecessary when numpy floating point error handling is set to print
potential micro-optimization when error handling is set to print
alleviates (but does not fix) numpy#5856 deadlock with sub-interpreters
addressed comments and initial test failed

Change-Id: I0414df8c5dca131e1f8c8d867791ba63cf992b63
@seberg
Copy link
Member

seberg commented Jul 23, 2015

OK, looks good to me, seems weird to grab the GIL, if we have an explicit print option, so merging. Thanks.

seberg added a commit that referenced this pull request Jul 23, 2015
ENH skip NPY_ALLOW_C_API for UFUNC_ERR_PRINT
@seberg seberg merged commit f09380a into numpy:master Jul 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants