xarray-contrib / cf-xarray

an accessor for xarray objects that interprets CF attributes
https://cf-xarray.readthedocs.io/
Apache License 2.0
157 stars 39 forks source link

Possible bug with `get_bounds_dim_name` #442

Closed huard closed 11 months ago

huard commented 1 year ago

This change in 0.8.1 seems problematic.

If the returned list has more than one item, it will raise a ValueError:

File /opt/conda/envs/birdy/lib/python3.9/site-packages/cf_xarray/accessor.py:2276, in CFDatasetAccessor.get_bounds_dim_name(self, key)
   2263 def get_bounds_dim_name(self, key: Hashable) -> Hashable:
   2264     """
   2265     Get bounds dim name for variable corresponding to key.
   2266 
   (...)
   2274     str
   2275     """
-> 2276     (crd_name,) = apply_mapper(_get_all, self._obj, key, error=False, default=[key])
   2277     variables = self._obj._variables
   2278     crd = variables[crd_name]

ValueError: too many values to unpack (expected 1) 

In my particular case, the returned value is ['lon_bounds', 'lon'].

dcherian commented 1 year ago

Can you provide an example please

sol1105 commented 12 months ago

@dcherian An example would be

import cf_xarray
from cf_xarray import datasets
bnds_dim = datasets.vert.cf.get_bounds_dim_name('longitude')

which leads to the problem described above.

larsbuntemeyer commented 11 months ago

Just also encounterd this problem, seems that cfxarray recognizes the bounds coordinates, e.g., in datasets.vert, as longitude/latitude coordinates since they have the units attribute defined as `degress*`, e.g.,

from cf_xarray.datasets import vert

vert.cf
...
      CF Coordinates:   longitude: ['lon', 'lon_bnds']
                        latitude: ['lat', 'lat_bnds']
                      * time: ['time']
                        vertical: n/a
...

That's why the mapper with the longitude key returns both values, hence the too many values to unpack... After dropping the units attribute, it works:

from cf_xarray.datasets import vert

# this works
vert.cf.get_bounds_dim_name('lon')

# using standard name fails
#vert.cf.get_bounds_dim_name('longitude')

# dropping the units attribute in the bounds works
vert_drop = vert.copy()
del vert_drop.lon_bnds.attrs['units']
vert_drop.cf.get_bounds_dim_name('longitude')
larsbuntemeyer commented 11 months ago

I wonder, if we could switch back to using the accessor itself, e.g., something like this?

crd = self._obj.cf[key]
bounds = self._obj.cf.get_bounds(key)

That would work, although i am still not sure, if those bounds should appear in the CF Coordinates print above?

dcherian commented 11 months ago

I wonder, if we could switch back to using the accessor itself, e.g., something like this?

This ends up being a quadratic loop over variables. This bug stems from my removing that quadratic loop :)

huard commented 11 months ago

Thanks !