-
Notifications
You must be signed in to change notification settings - Fork 50
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
RFC: add support for a dtype
kwarg to fftfreq
and rfftfreq
#717
Comments
Actually all the fft functions have this "type promotion" line, except for |
Oh I see it's because it's promoting from real to complex. It's a bit odd to call this type promotion, though, and it's even more awkward for functions like |
That sounds like an improvement - although I'd clarify the "same bit width" phrase a bit, since it'd be |
Sorry, Aaron, for some reason I missed this issue. My bad. For all applications that I know of (mainly in signal processing where fftfreq is used to prepare signal filters/windows), they must stay real. I think if (r)fftfreq were to return complex arrays, they'd break SciPy and Matplotlib in some unexpected ways. IIRC @oleksandr-pavlyk raised the same question about how return real types should be determined, and the argument back then involved viewing these two functions as constructors. I think one could argue the dtypes can be determined by the argument |
My point for adding a dtype argument wasn't to allow the user to specify a complex dtype. That should probably disallowed (or unspecified?). It was to allow the user to specify 32-bit vs. 64-bit, for consistency with the remaining creation functions in the spec. I don't think we can get the dtype from Given that this would be a new API, and no one has requested this, I would suggest making this change for 2024. We are already close to tagging 2023. And furthermore, this would not be compatible with the current NumPy API, which does not have a dtype argument in fftfreq or rfftfreq. If we were to squeeze this in for 2023, we would also want to get that added to NumPy for numpy 2.0 (which is probably possible, but again, no one has actually requested this yet, so it's not a priority). |
Sounds good to me, I have no concern for either v2023 or v2024. |
PR-URL: #720 Closes: #717 Closes: #718 Ref: #189 Co-authored-by: Leo Fang <[email protected]> Co-authored-by: Athan Reines <[email protected]> Reviewed-by: Leo Fang <[email protected]> Reviewed-by: Athan Reines <[email protected]> Reviewed-by: Ralf Gommers <[email protected]>
dtype
kwarg to fftfreq
and rfftfreq
The specs for fftfreq and rfftfreq say
"The returned array must have a real-valued floating-point data type determined by Type Promotion Rules."
But there is no type promotion here, since none of the input arguments are arrays. Presumably it should say that the return type should have the default real floating-point dtype.
But that also begs the question of whether these functions should have a
dtype
argument like the other array creation functions.The text was updated successfully, but these errors were encountered: