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

MNT log1p -> log in cython files #11852

Merged
merged 1 commit into from
Aug 18, 2018
Merged

MNT log1p -> log in cython files #11852

merged 1 commit into from
Aug 18, 2018

Conversation

qinhanmin2014
Copy link
Member

Revert the cython part of #11428
Will merge when green.

@rth
Copy link
Member

rth commented Aug 18, 2018

With respect to the error mentioned in #11848, I think the issue is that MSVC didn't include log1p in older versions (cf e.g. rust-lang/rust#29479). This is something we can revert once python 2 support is dropped (as Python 3 uses a newer version of MSVC on windows).

Copy link
Member

@rth rth left a comment

Choose a reason for hiding this comment

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

Thanks !

@jakirkham
Copy link
Contributor

FWIW NumPy handles the problem by defining log1p when it is not available. Cython often exports NumPy C functions like this. So we could use those instead to get the best of both worlds (e.g. log1p when system provided and a custom C function when not).

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.

3 participants