Move Frequencies, fftfreq, rfftfreq from DSP.jl to AbstractFFTs.jl

[carstenbauer]

See discussions in #19 and JuliaDSP/DSP.jl#298.

I have basically copy pasted the relevant code from DSP.jl and modified it according to @stevengj's comment.

This way one can use, for example, a measurement as sampling frequency:

julia> m = 5 ± 1
5.0 ± 1.0

julia> fftfreq(10, m)
10-element Frequencies{Measurement{Float64}}:
  0.0 ± 0.0
  0.5 ± 0.1
  1.0 ± 0.2
  1.5 ± 0.30000000000000004
  2.0 ± 0.4
 -2.5 ± 0.5
 -2.0 ± 0.4
 -1.5 ± 0.30000000000000004
 -1.0 ± 0.2
 -0.5 ± 0.1

Closes #19.

[carstenbauer]

Note that a quantity from Unitful.jl doesn't currently work due to

julia> u = 3 * Unitful.Hz
3 Hz

julia> u isa Real
false

as has been indicated here. Should we generalize to <:Number instead of <:Real?

(I decided to make the change until someone complains.)

[giordano]

So now a number with both physical units and uncertainty should work, right? 😃

[carstenbauer]

So now a number with both physical units and uncertainty should work, right? 😃

Yes.

julia> fftfreq(4, (5 ± 1) * Unitful.Hz)
4-element Frequencies{Quantity{Measurement{Float64},�^-1,Unitful.FreeUnits{(Hz,),�^-1,nothing}}}:
    0.0 ± 0.0 Hz
  1.25 ± 0.25 Hz
   -2.5 ± 0.5 Hz
 -1.25 ± 0.25 Hz

[carstenbauer]

Maybe the optional sampling frequency should rather be a keyword argument (in the spirit of #15)?

Otherwise this should be good to go.

[stevengj]

Oh, I see the reason for assigning the Nyquist element (the middle frequency for even n) to be a "negative" frequency — it is consistent with what fftshift does.

Of course, it's completely arbitrary whether we call this frequency positive or negative, but it's annoying that it is inconsistent with the real-input FFT case where we call all the frequencies nonnegative. But I guess we can't change the fftshift function at this point. 😢

[carstenbauer]

Ready for merge?

[galenlynch]

It might be worth thinking about how this change affects other packages. E.g. how will this affect DSP.jl, where this code was taken from, and do other packages depend on these functions being in DSP.jl? It would be great if you could also open PRs against other affected packages to accommodate this change.

[carstenbauer]

It might be worth thinking about how this change affects other packages.

Well, this PR is only adding a new feature. Therefore, technically, no other package is affected - the PR is trivially non-breaking. Maybe DSP.jl is an exception because I took the code from there and we probably want to avoid multiple identical definitions in different packages. I'll create a PR over there to switch to using AbstractFFTs fftfreq etc. in DSP.jl. However, technically this shouldn't affect any other packages either as this change will also be non-breaking (right?).

If you're suggesting to make PRs against all packages that use DSP.jl for the sole purpose of getting fftfreq and companions, this sounds like quite a bit of work, which I'm not sure is worth doing, since these packages will this work fine. How would I even find those packages? Any ideas?

[carstenbauer]

DSP.jl PR created: JuliaDSP/DSP.jl#300

However, it can only be incorporated after this PR here has been merged.

[galenlynch]

Maybe it would be enough to have DSP.jl still export rfftfreq, fftfreq, etc. but add some deprecation warning to let users know that its location will be changing?

[heitorPB]

Is it possible to have a new release of AbstractFFTs with this PR?

[stevengj]

Just need a PR to update Project.toml, I think?

[galenlynch]

@stevengj Could you please tag a new version so we can fix the consequences of this in DSP.jl (JuliaDSP/DSP.jl#306)?