Skip to content

Upgrade to easyfft v0.3 #3

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

WalterSmuts
Copy link
Contributor

Unfortunately this requires an extra Default trait bound which results in requiring a major version change for fftconvolve.

Practically it's not an issue since all elements that you'd want to convolve would implement Default, but it's a bit annoying.

Default is required for initializing the scratch-buffer, which is strictly not necessary but requires changes to rustfft. It's currently pending this issue in rustfft:
ejmahler/RustFFT#105

Unfortunately this requires an extra Default trait bound which results
in requiring a major version change for fftconvolve.

Practically it's not an issue since all elements that you'd want to
convolve would implement Default, but it's a bit annoying.

Default is required for initializing the scratch-buffer, which is
strictly not necessary but requires changes to `rustfft`. It's currently
pending this issue in rustfft:
ejmahler/RustFFT#105
@WalterSmuts WalterSmuts changed the title Update to easyfft v0.3 Upgrade to easyfft v0.3 Dec 9, 2022
@rhysnewell
Copy link
Owner

Easy as, but should this wait until that RustFFT issue is resolved or do you reckon it is good to go now?

@WalterSmuts
Copy link
Contributor Author

I don't think the issue will be resolved anytime soon so I would recommend to merge for now. Unfortunately it means a major version change for fftconvolve so consumers would have to manually update to the newer version and resolve any conflicts. Since fftconvolve is still relatively young I don't think it's much of a concern. The chances for running into upgrade conflicts is also super slim since the only change is an added Default trait bound, which all applications would likely already conform to.

If the issue get's resolved, it would mean easyfft and fftconvolve could drop the Default requirement without a major version change.

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.

2 participants