Skip to content
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

Incorrect results when using xarray.ufuncs.angle(..., deg=True) #5857

Closed
cvr opened this issue Oct 12, 2021 · 4 comments · Fixed by #6491
Closed

Incorrect results when using xarray.ufuncs.angle(..., deg=True) #5857

cvr opened this issue Oct 12, 2021 · 4 comments · Fixed by #6491

Comments

@cvr
Copy link

cvr commented Oct 12, 2021

What happened:

The xarray.ufuncs.angle is broken. From the help docstring one may use option deg=True to have the result in degrees instead of radians (which is consistent with numpy.angle function). Yet results show that this is not the case. Moreover specifying deg=True or deg=False leads to the same result with the values in radians.

What you expected to happen:

To have the result of xarray.ufuncs.angle converted to degrees when option deg=True is specified.

Minimal Complete Verifiable Example:

# Put your MCVE code here
import numpy as np
import xarray as xr

ds = xr.Dataset(coords={'wd': ('wd', np.arange(0, 360, 30, dtype=float))})

Z = xr.ufuncs.exp(1j * xr.ufuncs.radians(ds.wd))
D = xr.ufuncs.angle(Z, deg=True)  # YIELDS INCORRECT RESULTS
if not np.allclose(ds.wd, (D % 360)):
    print(f"Issue with angle operation: {D.values%360} instead of {ds.wd.values}" \
        + f"\n\tERROR   xr.ufuncs.angle(Z, deg=True) gives incorrect results !!!")

D = xr.ufuncs.degrees(xr.ufuncs.angle(Z))  # Works OK
if not np.allclose(ds.wd, (D % 360)):
    print(f"Issue with angle operation: {D%360} instead of {ds.wd}" \
    + f"\n\tERROR   xr.ufuncs.degrees(xr.ufuncs.angle(Z)) gives incorrect results!!!")

D = xr.apply_ufunc(np.angle, Z, kwargs={'deg': True})  # Works OK
if not np.allclose(ds.wd, (D % 360)):
    print(f"Issue with angle operation: {D%360} instead of {ds.wd}" \
    + f"\n\tERROR   xr.apply_ufunc(np.angle, Z, kwargs={{'deg': True}}) gives incorrect results!!!")

Anything else we need to know?:

Though xarray.ufuncs has a deprecated warning stating that the numpy equivalent may be used, this is not true for numpy.angle. Example:

import numpy as np
import xarray as xr

ds = xr.Dataset(coords={'wd': ('wd', np.arange(0, 360, 30, dtype=float))})

Z = np.exp(1j * np.radians(ds.wd))
print(Z)
print(f"Is Z an XArray? {isinstance(Z, xr.DataArray)}")

D = np.angle(ds.wd, deg=True)
print(D)
print(f"Is D an XArray? {isinstance(D, xr.DataArray)}")

If this code is run, the result of numpy.angle(xarray.DataArray) is not a DataArray object, contrary to other numpy operations (for all versions of xarray I've used). Hence the xarray.ufuncs.angle is a great option, if it was not for the current problem.

Environment:

No issues with xarray versions 0.16.2 and 0.17.0. This error happens from 0.18.0 onwards, up to 0.19.0 (recentmost).

Output of xr.show_versions()

INSTALLED VERSIONS

commit: None
python: 3.7.10 (default, Feb 26 2021, 18:47:35)
[GCC 7.3.0]
python-bits: 64
OS: Linux
OS-release: 4.19.0-18-amd64
machine: x86_64
processor:
byteorder: little
LC_ALL: en_US.utf8
LANG: C.UTF-8
LOCALE: ('en_US', 'UTF-8')
libhdf5: None
libnetcdf: None

xarray: 0.19.0
pandas: 1.2.3
numpy: 1.20.2
scipy: 1.5.3
netCDF4: None
pydap: None
h5netcdf: None
h5py: None
Nio: None
zarr: None
cftime: None
nc_time_axis: None
PseudoNetCDF: None
rasterio: None
cfgrib: None
iris: None
bottleneck: None
dask: None
distributed: None
matplotlib: None
cartopy: None
seaborn: None
numbagg: None
pint: None
setuptools: 58.2.0
pip: 21.3
conda: 4.10.3
pytest: None
IPython: None
sphinx: None

@TomNicholas
Copy link
Member

TomNicholas commented Oct 12, 2021

The result of numpy.angle(xarray.DataArray) is not a DataArray object, contrary to other numpy operations

This is because np.angle is not a ufunc - the cleanest solution to this should be to fix that issue in numpy.

EDIT: Of course that doesn't explain why xarray.ufuncs.angle is behaving incorrectly - it just means that if we fixed the upstream issue in numpy then no-one need use xarray.ufuncs.angle.

@vincentschut
Copy link

While closed, this is still an issue for us. np.angle is still not a ufunc, and when applied to an xr.DataArray will return a np array. I understand the removal of the xr.ufuncs namespace, but we're stuck with xarray 2022.3.0 now because upgrading will break our code. What is the recommended workaround here for code that depends on xr.ufuncs.angle?

@TomNicholas TomNicholas reopened this Sep 12, 2022
@TomNicholas
Copy link
Member

np.angle is still not a ufunc

Yes it would be nice if this were fixed upstream. (Would you be interested in having a go?)

What is the recommended workaround here for code that depends on xr.ufuncs.angle?

Perhaps you could make a custom angle function that does behave like a ufunc, maybe like the example given here?

@dcherian dcherian removed the bug label Sep 12, 2022
@max-sixty
Copy link
Collaborator

Closing as upstream issue — xarray.ufuncs no longer exists in xarray...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants