Skip to content

Conversation

@mhvk
Copy link
Contributor

@mhvk mhvk commented Apr 27, 2024

This is a slightly improved fix for #26349, which changes the code to actually do what a comment stated it would, making the fix from #26354 no longer necessary. Of course, it keeps the tests, so that we can be sure the fix actually is correct.

Ideally, backported for 2.0.0 as well.

@mhvk mhvk added component: numpy.fft 03 - Maintenance 09 - Backport-Candidate PRs tagged should be backported C++ Related to introduction and use of C++ in the NumPy code base labels Apr 27, 2024
@mhvk mhvk added this to the 2.0.0 release milestone Apr 27, 2024
@mhvk mhvk requested review from WarrenWeckesser and charris April 27, 2024 15:34
@charris charris merged commit 3d33d5f into numpy:main Apr 27, 2024
Copy link
Member

@WarrenWeckesser WarrenWeckesser left a comment

Choose a reason for hiding this comment

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

Thanks for fixing my fix!

@charris
Copy link
Member

charris commented Apr 27, 2024

Thanks Marten. I'd need to look deeper into the code to be sure how this works, but otherwise it looks reasonable and the tests pass, so in it goes.

@mhvk
Copy link
Contributor Author

mhvk commented Apr 27, 2024

Thanks Marten. I'd need to look deeper into the code to be sure how this works, but otherwise it looks reasonable and the tests pass, so in it goes.

It's all in copy_input - which zeros everything beyond where the input goes. But the bug was that I copied the whole input even if fewer points were requested,

@mhvk mhvk deleted the fft-rfft-n-even-bugfix-replacement branch April 27, 2024 17:51
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

03 - Maintenance C++ Related to introduction and use of C++ in the NumPy code base component: numpy.fft

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants