Open
Conversation
Member
|
Not a full review, will try to give this a read tomorrow.
Most likely a left over from the times before
Good question, not sure. I think I'd also prefer having the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR contains the implementation discussed in #534. The
npyffimodule has been updated to match the NumPy implementation of ABI v2 when theNPY_FEATURE_VERSIONmacro is set to 0x0000000c (v1.15). Some new macros are required for the implementation, I prefer to follow the same layout of NumPy to make easier future changes.The major changes of this PR are:
NonNullmarker is used to store the pointer contained in capsules.T([u8; 0]).npy_longdoublehave been commented; previously they were related tof64, but on some platforms long double is 80-bit extended precision.During the implementation, some doubts arose:
std::os::rawused? Usuallystd::ffiis preferred.Option<...>? I found that the usage ofOption<...>is more common in member definitions, probably even inpyo3. For example: