xgcm / xrft

Fourier transforms on xarray data structures
https://xrft.readthedocs.io/en/latest/
MIT License
153 stars 45 forks source link

Refactor duplicated code in xrft.py into separate functions; add one test; format with black #196

Closed simonkeys closed 1 year ago

simonkeys commented 1 year ago

I'm thinking of adding some features to xrft, so I started out by doing some refactoring. I added one test to check the case of ifft with unsorted coordinates.

If this is too many changes at once, I can break it up into more PRs with fewer commits.

roxyboy commented 1 year ago

Thanks a lot for this refactoring @simonkeys ! Just out of curiosity, what features are you planning to add?

simonkeys commented 1 year ago

Basically whatever comes up in the project I'm using it for (image processing for interferometric microscopy). So far I'm thinking

roxyboy commented 1 year ago
  • interoperability with pint-xarray
  • function returning the coordinates that would appear in a fft
  • possible option to back with scipy fft library instead of numpy (maybe a speed-up? I'm not sure about that)
  • option to do a fft with a real dimension but then return negative and positive frequencies instead of just positive

All features you suggest would be a great addition to xrft. Regarding scipy over numpy, this is something I've thought about during the early stages of developing xrft but do you see any clear advantages of one over the other? As far as I understand, xarray depends heavily on numpy so this would be an argument to stick with numpy.

roxyboy commented 1 year ago

All the checks passed so I'm going to merge this PR. I'd be happy to continue the discussion if you could raise an issue on Github :)