Skip to content

Handling of non-string dimension names #5148

Open

Description

@bcbnz

While working on a pull request (#5149) for #5146 I came across an inconsistency in allowed dimension names. If I try and create a DataArray with a non-string dimension, I get a TypeError:

>>> import xarray as xr
>>> da = xr.DataArray(np.ones((5, 5)), dims=[1, "y"])
...
TypeError: dimension 1 is not a string

But creating it with a string and renaming it works:

>>> da = xr.DataArray(np.ones((5, 5)), dims=["x", "y"]).rename(x=1)
>>> da
<xarray.DataArray (1: 5, y: 5)>
array([[1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1.]])
Dimensions without coordinates: 1, y

I can create a dataset via this renaming, but trying to get the repr value fails as xarray.core.utils.SortedKeysDict tries to sort it and cannot compare the string dimension to the int dimension:

>>> import xarray as xr
>>> ds = xr.Dataset({"test": xr.DataArray(np.ones((5, 5)), dims=["x", "y"]).rename(x=1)})
>>> ds
...
~/software/external/xarray/xarray/core/formatting.py in dataset_repr(ds)
    519 
    520     dims_start = pretty_print("Dimensions:", col_width)
--> 521     summary.append("{}({})".format(dims_start, dim_summary(ds)))
    522 
    523     if ds.coords:

~/software/external/xarray/xarray/core/formatting.py in dim_summary(obj)
    422 
    423 def dim_summary(obj):
--> 424     elements = [f"{k}: {v}" for k, v in obj.sizes.items()]
    425     return ", ".join(elements)
    426 

~/software/external/xarray/xarray/core/formatting.py in <listcomp>(.0)
    422 
    423 def dim_summary(obj):
--> 424     elements = [f"{k}: {v}" for k, v in obj.sizes.items()]
    425     return ", ".join(elements)
    426 

/usr/lib/python3.9/_collections_abc.py in __iter__(self)
    847 
    848     def __iter__(self):
--> 849         for key in self._mapping:
    850             yield (key, self._mapping[key])
    851 

~/software/external/xarray/xarray/core/utils.py in __iter__(self)
    437 
    438     def __iter__(self) -> Iterator[K]:
--> 439         return iter(self.mapping)
    440 
    441     def __len__(self) -> int:

~/software/external/xarray/xarray/core/utils.py in __iter__(self)
    504     def __iter__(self) -> Iterator[K]:
    505         # see #4571 for the reason of the type ignore
--> 506         return iter(sorted(self.mapping))  # type: ignore[type-var]
    507 
    508     def __len__(self) -> int:

TypeError: '<' not supported between instances of 'str' and 'int'

The same thing happens if I call rename on the dataset rather than the array it is initialised with.

If the initialiser requires the dimension names to be strings, and other code (which includes the HTML formatter I was looking at when I found this) assume that they are, then rename and any other method which can alter dimension names should also enforce the string requirement.

Environment:

Output of xr.show_versions()

INSTALLED VERSIONS

commit: 851d85b
python: 3.9.2 (default, Feb 20 2021, 18:40:11)
[GCC 10.2.0]
python-bits: 64
OS: Linux
OS-release: 5.11.13-arch1-1
machine: x86_64
processor:
byteorder: little
LC_ALL: None
LANG: en_NZ.UTF-8
LOCALE: en_NZ.UTF-8
libhdf5: 1.12.0
libnetcdf: 4.7.4

xarray: 0.17.0
pandas: 1.2.3
numpy: 1.20.1
scipy: 1.6.2
netCDF4: 1.5.6
pydap: None
h5netcdf: 0.10.0
h5py: 3.2.1
Nio: None
zarr: None
cftime: 1.4.1
nc_time_axis: None
PseudoNetCDF: None
rasterio: 1.2.2
cfgrib: None
iris: None
bottleneck: 1.3.2
dask: 2021.03.0
distributed: 2021.03.0
matplotlib: 3.4.1
cartopy: 0.18.0
seaborn: 0.11.1
numbagg: None
pint: None
setuptools: 54.2.0
pip: 20.3.1
conda: None
pytest: 6.2.3
IPython: 7.22.0
sphinx: 3.5.4

Activity

  1. keewis commented on Apr 13, 2021

    @keewis
    Collaborator

    we do want to allow general hashables to mirror the general dict object (and we do need it for some methods, like to_unstacked_dataset), but this is obviously nothing well tested. The main issue is that right now we sort Dataset dims and would need to be able to convert the keys to something comparable. See also #2173, where I suggested to convert to str. This might be too naive for immutable objects without a custom __str__ / __repr__, so something like this, maybe:

    sorted(self.mapping, key=lambda k: (type(k).__qualname__, str(k)))

    which would group objects with the same type name together

  2. bcbnz commented on Apr 13, 2021

    @bcbnz
    ContributorAuthor

    This is also something that the backends will need to consider:

    >>> xr.DataArray(np.ones(5)).rename(dim_0=1).to_netcdf("/tmp/test.nc")
    ...
    /usr/lib/python3.9/site-packages/xarray/backends/netCDF4_.py in set_dimension(self, name, length, is_unlimited)
        439     def set_dimension(self, name, length, is_unlimited=False):
        440         dim_length = length if not is_unlimited else None
    --> 441         self.ds.createDimension(name, size=dim_length)
        442 
        443     def set_attribute(self, key, value):
    
    src/netCDF4/_netCDF4.pyx in netCDF4._netCDF4.Dataset.createDimension()
    
    src/netCDF4/_netCDF4.pyx in netCDF4._netCDF4.Dimension.__init__()
    
    TypeError: expected bytes, int found

    I guess they could either convert to string (which will result in a different array when loaded) or a better error message (netCDF files only support strings for dimension names or similar).

  3. bcbnz commented on Apr 13, 2021

    @bcbnz
    ContributorAuthor

    Should I change the title here to be more general or would you prefer to close this and continue discussion in the other issue you linked?

  4. keewis commented on Apr 13, 2021

    @keewis
    Collaborator

    I guess they could either convert to string or a better error message

    I think improving the error message is the way to go.

    would you prefer to close this

    I think we can keep the discussion here

  5. changed the title [-]Dimensions must be strings when created, but can be renamed to non-strings[/-] [+]Handling of non-string dimension names[/+] on Apr 13, 2021
  6. bcbnz commented on Apr 13, 2021

    @bcbnz
    ContributorAuthor

    I've changed the title.

    I think improving the error message is the way to go.

    Agreed. Just playing with it, the attributes also need to be strings for netCDF (I'm not familiar with the other backends) so an equivalent error message should be added for them. There is already a good error message for attributes, at least for netCDF, I wasn't looking at it properly.

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions