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

API: Replicate numpy's fftn behaviour when s is given but not axes #10594

Merged
merged 3 commits into from Aug 5, 2019

Conversation

@peterbell10
Copy link
Member

peterbell10 commented Aug 5, 2019

Reference issue

Closes gh-10588

What does this implement/fix?

This updates the helper function _init_nd_shape_and_axes to allow len(s) < x.ndim when axes=None. I've also added extra checks to fftpack to ensure it keeps the old behaviour.

Additional information

I've also noticed another difference from numpy fft:

Repeated indices in axes means that the inverse transform over that
axis is performed multiple times.

whereas scipy.fft will complain about axes not being unique. In this case I'm not convinced that NumPy's behaviour is better though. It seems like something more likely to just be a mistake and I also know that pyfftw doesn't handle this correctly. So, I've just removed that comment from the docs.

@grlee77
grlee77 approved these changes Aug 5, 2019
Copy link
Contributor

grlee77 left a comment

The shape/axis handling looks good.

I am fine with not allowing repeated axes unless someone else has a good use case for that? @larsoner: Does that sound okay to you as well?

@larsoner

This comment has been minimized.

Copy link
Member

larsoner commented Aug 5, 2019

Yes I think repeated axes are much more likely errors, so I'm fine with not allowing it

@grlee77 grlee77 added the scipy.fft label Aug 5, 2019
@grlee77 grlee77 added this to the 1.4.0 milestone Aug 5, 2019
@larsoner

This comment has been minimized.

Copy link
Member

larsoner commented Aug 5, 2019

Sorry @peterbell10, another merge conflict for you

@peterbell10 peterbell10 force-pushed the peterbell10:auto-axes branch from 6811740 to 34f5ec8 Aug 5, 2019
@peterbell10

This comment has been minimized.

Copy link
Member Author

peterbell10 commented Aug 5, 2019

Okay, rebased.

@larsoner larsoner merged commit 09efbc2 into scipy:master Aug 5, 2019
9 checks passed
9 checks passed
ci/circleci: build_docs Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
scipy.scipy Build #20190805.10 succeeded
Details
scipy.scipy (Linux_Python_36_32bit_full) Linux_Python_36_32bit_full succeeded
Details
scipy.scipy (Windows Python35-64bit-full) Windows Python35-64bit-full succeeded
Details
scipy.scipy (Windows Python36-32bit-full) Windows Python36-32bit-full succeeded
Details
scipy.scipy (Windows Python36-64bit-full) Windows Python36-64bit-full succeeded
Details
scipy.scipy (Windows Python37-64bit-full) Windows Python37-64bit-full succeeded
Details
@larsoner

This comment has been minimized.

Copy link
Member

larsoner commented Aug 5, 2019

Thanks @peterbell10

@peterbell10 peterbell10 deleted the peterbell10:auto-axes branch Aug 5, 2019
grlee77 added a commit to grlee77/pyFFTW that referenced this pull request Sep 5, 2019
grlee77 added a commit to pyFFTW/pyFFTW that referenced this pull request Oct 10, 2019
…ment (#274)

* ENH: add workers argument to scipy_fft interfaces.

remove the equivalent threads argument as it is no longer needed

* TST: update scipy.fft threading tests to use 'workers' instead

* scipy_fft interface: update shape handling to match scipy/scipy#10594

* TST: add missing import of scipy.signal

* TST: update numpy module import logic in test_pyfftw_base.py for numpy 1.17

* DOC: fix codeblock->code-block spelling to avoid unknown directive warnings

* DOC: avoid sphinx warning that html_static_path entry does not exist

* DOC: update additional parameters section of interfaces docs

* DOC: fix link in tutorial.rst
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.