search

ejmahler / RustFFT

RustFFT is a high-performance FFT library written in pure Rust.
Apache License 2.0
678 stars 47 forks source link

Remove unnecessary num_traits re-export #95

Closed WalterSmuts closed 1 year ago

WalterSmuts commented 1 year ago

Nothing in the external API requires the consumer to use num_traits directly. Removing this will decouple the rustfft major version from the the num_traits major version.

FftNum is defined in terms of the FromPrimitive and Signed traits in the num_traits crate however, so whenever they undergo a breaking change, rustfft will still require a major version change.

Since this is a possible breaking change for consumers that happen to consume num_traits via rustfft, this would require a major version bump.

ejmahler commented 1 year ago

Looks like this failed because it needs the neon PR. No big deal.

I think this may be a holdover from before FftNum existed - long ago, the trait bounds were specified directly in terms of num_traits traits, and I ended up adding FftNum purely as a way to make the trait bounds on functions simpler.

I'm definitely down to simplify the library, so I'm open to this change. It's a breaking change, so it will have to wait until major changes are allowed again in 2024. I'm fine with leaving this open until then.

HEnquist commented 1 year ago

Somewhat related: https://github.com/rust-num/num/issues/421 It bothers me a little that the num crates, that are very mature and stable, and used basically everywhere, are still at pre-1.0 versions.

WalterSmuts commented 1 year ago

So I went ahead and copied rustfft and realfft's examples in easyfft. My thinking is that, while only exporting the required structs (Complex in this case), it's pretty useless if you can't use the related definitions. So even though it may result in less breaking changes, it's not worth it IMO.

ejmahler commented 1 year ago

Thanks for continuing to investigate, and thanks for making the recommendation even though it didn't work out!