Skip to content

Rename scipy.fftpack to scipy.fft#201

Open
Rotzbua wants to merge 1 commit intoJohannesBuchner:masterfrom
Rotzbua:rename_scipy
Open

Rename scipy.fftpack to scipy.fft#201
Rotzbua wants to merge 1 commit intoJohannesBuchner:masterfrom
Rotzbua:rename_scipy

Conversation

@Rotzbua
Copy link
Copy Markdown

@Rotzbua Rotzbua commented Oct 24, 2023

Change

  • Rename scipy.fftpack to scipy.fft

Reference

This submodule is considered legacy and will no longer receive updates. This could also mean it will be removed in future SciPy versions. New code should use scipy.fft.

Source: https://docs.scipy.org/doc/scipy/reference/fftpack.html

Closes #108

@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 87.847%. remained the same when pulling bccedd0 on Rotzbua:rename_scipy into 5e9ef5d on JohannesBuchner:master.

@JohannesBuchner
Copy link
Copy Markdown
Owner

Will this make imagehash break on installs with old scipy versions?

@Rotzbua
Copy link
Copy Markdown
Author

Rotzbua commented Oct 24, 2023

What means and is "old"? If that is important, shouldn't it be part already of the tests?

@JohannesBuchner
Copy link
Copy Markdown
Owner

To have this accepted, we would need to know

  • in which scipy version the rename was introduced,
  • at what version the name scipy.fft became available.
  • which year those versions were released.
  • which python version that year corresponds to.

Currently nothing is broken and the code is passing tests, so there is little gain by this PR. There is merely a vague threat of potential (not even planned) future incompatibility. Scipy advises new code should use scipy.fft. imagehash is not new code.

@JohannesBuchner
Copy link
Copy Markdown
Owner

For example, I am on Python 3.11.6 with scipy 1.10.1 and can import fftpack without issue/warning.

@rosanzheng
Copy link
Copy Markdown

Hi, I checked roughly and can answer a part of your questions:

  1. in which scipy version the rename was introduced
    Renaming was introduced in version 1.8.0, based on this GitHub issue and the following changelog:
    https://docs.scipy.org/doc/scipy/release/1.8.0-notes.html
    DEP: Deprecate private namespace in scipy.fftpack scipy/scipy#14913

  2. at what version the name scipy.fft became available.
    scipy.fft became available in 1.4.0 according to the release notes
    https://docs.scipy.org/doc/scipy/release/1.4.0-notes.html

  3. which year those versions were released
    1.8.0 seems to have been released on Feb 2022 https://github.com/scipy/scipy/milestone/58
    1.4.0 seems to have been released on Dec 2019 https://github.com/scipy/scipy/milestone/40

For the Python version i need to check more, hope I could help a little bit for starters!

@JohannesBuchner
Copy link
Copy Markdown
Owner

I am still confused though. Does using scipy.fftpack in imagehash currently make problems? I thought both are ok at the moment?

If there is no problem currently, I'd prefer to wait until scipy disables it, and then make the change in imagehash, to support as long as possible old scipy/python combinations without breaking them.

This submodule is considered legacy and will no longer receive updates. This could also mean it will be removed in future SciPy versions. New code should use scipy.fft.
https://docs.scipy.org/doc/scipy/reference/fftpack.html
@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 87.063%. remained the same
when pulling f8bd5b6 on Rotzbua:rename_scipy
into 07951cd on JohannesBuchner:master.

@jorenham
Copy link
Copy Markdown

From the scipy.fftpack docs:

This submodule is considered legacy and will no longer receive updates. While we currently have no plans to remove it, we recommend that new code uses more modern alternatives instead. New code should use scipy.fft.

And if you compare fft.dct and fftpack.dct, you'll see that the api of fft.dct has additional parameters such as workers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

scipy.fftpack was renamed to scipy.fft

5 participants