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

BUG: Use C linkage for random distributions #18264

Merged
merged 2 commits into from
Feb 2, 2021

Conversation

mckib2
Copy link
Contributor

@mckib2 mckib2 commented Jan 31, 2021

When using the numpy.random C API with Cython as described in examples with C++ sources instead of C sources, C++ name mangling will prevent correctly linking to symbols in the npyrandom static library which has C linkage. A work-around is described here, but it seems like adding some extern "C" declarations should do the trick. These are found in nearly all other numpy header files, e.g. npy_math.h.

Someone else may need to comment on why cffi extending tests are failing, I am unfamiliar with cffi. EDIT: filtering out extern "C" statements was enough to get the tests working

@charris charris changed the title FIX: random: C linkage for random distributions BUG: Use C linkage for random distributions Jan 31, 2021
@charris charris added 09 - Backport-Candidate PRs tagged should be backported component: numpy.random labels Jan 31, 2021
@charris charris added this to the 1.20.1 release milestone Jan 31, 2021
with open(os.path.join(inc_dir, 'random', 'distributions.h')) as fid:
s = []
in_skip = 0
ignoring = False
Copy link
Member

Choose a reason for hiding this comment

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

Why do the guards need to be stripped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't say (not familiar with cffi), but the tests fail when they're there and pass when they're not. I assume it's for the same reason that include statements (and friends) are stripped

@charris charris merged commit b3a4438 into numpy:master Feb 2, 2021
@charris
Copy link
Member

charris commented Feb 2, 2021

Thanks @mckib2 .

@mckib2 mckib2 deleted the c-extern-npyrandom branch February 2, 2021 23:11
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Feb 4, 2021
@charris charris removed this from the 1.20.1 release milestone Feb 4, 2021
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

2 participants