search

xgcm / xrft

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

Update `axis_num` when `detrend='linear'` is taken #160

Closed roxyboy closed 3 years ago

roxyboy commented 3 years ago

This addresses issue #159 and updates the axis_num when detrend='linear' because the latter changes the order of the dimensions.

lanougue commented 3 years ago

Hi @roxyboy. Instead of updating axis_num after detrending, I would rather transpose the output of detrending into the previous/old axis number. This would ensure compatibility with potential other variables defined previously with the initial order (like the "N" variable used in _freq() function after detrending). As it is now, I think it can create errors

roxyboy commented 3 years ago

Not sure about this because da's dims order is altered when real_dim is not None (see some code lines before your changes). I would save da's dims order just before detrending and transpose just after it.

dims_order = da.dims
da = _detrend(da, ...).transpose(*dims_order)

I agree, and that's why I've changed the order to save rawdims after the real_dim argument. rawdims and your dims_order should be the same with this PR.

lanougue commented 3 years ago

raw_dims definition (as it was before this change) ensures input and output have the same dims order. If you move raw_dims as you did, this behaviour will be broken because the real dim will always be moved to the end

roxyboy commented 3 years ago

raw_dims definition (as it was before this change) ensures input and output have the same dims order. If you move raw_dims as you did, this behaviour will be broken because the real dim will always be moved to the end

Yea, you're correct. I added an orig_dims variable before detrending.

roxyboy commented 3 years ago

So... I guess I'll merge this.