-
-
Notifications
You must be signed in to change notification settings - Fork 19.1k
COMPAT: Change khcomplex64/128_t typedefs #54375
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
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
940a450
COMPAT: Change khcomplex64/128_t typedefs
mroeschke 65acbab
Merge remote-tracking branch 'upstream/main' into compat/np/imag
mroeschke c4571d9
Spacing, use double
mroeschke 1efe3be
Use double more places
mroeschke 89091d7
Go back to old fix
mroeschke dd12989
Check both, cant repo locally
mroeschke da70599
Merge remote-tracking branch 'upstream/main' into compat/np/imag
mroeschke a9c6a11
Rule out pandas inference
mroeschke 9d7cad0
Merge remote-tracking branch 'upstream/main' into compat/np/imag
mroeschke d32fbb5
Use np.nan insted of float(nan)
mroeschke File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will this change anything on platforms where float/double aren't 32/64 bits? (which i guess we have zero testing for so not a huge deal)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure. It appears in numpy/numpy#24085 in numpy/core/include/numpy/npy_math.h numpy also defines real/imag components as doubles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the misnomer is to call these complex64 and complex128. The C99 standard has float complex and double complex types, but nothing that is a fixed width
See also https://stackoverflow.com/questions/2524737/fixed-size-floating-point-types#2524933
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I guess technically float32/float64 are sized aliases under the assumption they refer to float/double, but in practice the assumption seems so strong that I doubt it matters.
I am very puzzled about the change though, this is exactly what it was after all, and I don't think C cares about type aliases (even if there was interaction anywhere). I am wondering if the
include <numpy/arrayobject.h>
somehow is involved, but I don't see why it should be either?I assume the value stored is correct here and just the printing somehow thinks it is printing a NaN?