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

DataArray types must be strings #8546

Closed
5 tasks done
maresb opened this issue Dec 12, 2023 · 2 comments · Fixed by #8559
Closed
5 tasks done

DataArray types must be strings #8546

maresb opened this issue Dec 12, 2023 · 2 comments · Fixed by #8559
Labels

Comments

@maresb
Copy link
Contributor

maresb commented Dec 12, 2023

What happened?

While trying to understand the way dimensions are typed in namedarray I was confused by the requirement that dimension names must be strings:

>>> da = xr.DataArray(data=[1,2,3], dims=[7])
TypeError: dimension 7 is not a string

However, the type hints suggest that dimension names can more generally be Hashable. According to @headtr1ck's post, the desired behavior is that Hashable dimension names should be allowed, so this TypeError may be considered a bug.

What did you expect to happen?

No response

Minimal Complete Verifiable Example

No response

MVCE confirmation

  • Minimal example — the example is as focused as reasonably possible to demonstrate the underlying issue in xarray.
  • Complete example — the example is self-contained, including all data and the text of any traceback.
  • Verifiable example — the example copy & pastes into an IPython prompt or Binder notebook, returning the result.
  • New issue — a search of GitHub Issues suggests this is not a duplicate.
  • Recent environment — the issue occurs with the latest version of xarray and its dependencies.

Relevant log output

No response

Anything else we need to know?

No response

Environment

INSTALLED VERSIONS

commit: None
python: 3.11.6 | packaged by conda-forge | (main, Oct 3 2023, 10:40:35) [GCC 12.3.0]
python-bits: 64
OS: Linux
OS-release: 5.15.0-89-generic
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: C.UTF-8
LANG: C.UTF-8
LOCALE: ('en_US', 'UTF-8')
libhdf5: 1.14.2
libnetcdf: 4.9.2

xarray: 2023.11.0
pandas: 1.5.3
numpy: 1.26.2
scipy: 1.11.4
netCDF4: 1.6.5
pydap: None
h5netcdf: 1.3.0
h5py: 3.10.0
Nio: None
zarr: 2.16.1
cftime: 1.6.3
nc_time_axis: None
iris: None
bottleneck: None
dask: 2023.11.0
distributed: 2023.11.0
matplotlib: 3.8.2
cartopy: 0.22.0
seaborn: 0.13.0
numbagg: None
fsspec: 2023.10.0
cupy: None
pint: None
sparse: 0.14.0
flox: 0.8.3
numpy_groupies: 0.10.2
setuptools: 68.2.2
pip: 23.3.1
conda: 23.10.0
pytest: 7.4.3
mypy: 1.7.1
IPython: 8.18.0
sphinx: None
/opt/conda/lib/python3.11/site-packages/_distutils_hack/init.py:33: UserWarning: Setuptools is replacing distutils.
warnings.warn("Setuptools is replacing distutils.")

@maresb maresb added bug needs triage Issue that has not been reviewed by xarray team member labels Dec 12, 2023
Copy link

welcome bot commented Dec 12, 2023

Thanks for opening your first issue here at xarray! Be sure to follow the issue template!
If you have an idea for a solution, we would really welcome a Pull Request with proposed changes.
See the Contributing Guide for more.
It may take us a while to respond here, but we really value your contribution. Contributors like you help make xarray better.
Thank you!

@max-sixty
Copy link
Collaborator

Yes, I'm not sure why that check is there. Probably it should be replaced with a check for Hashable — at least we could make the change that and confirm nothing breaks.

PRs welcome!

Notably it is possible to make a Dataset with a non-string dim name:

xr.Dataset(dict(x=((7,),[1,2,3])))
Out[4]:
<xarray.Dataset>
Dimensions:  (7: 3)
Dimensions without coordinates: 7
Data variables:
    x        (7) int64 1 2 3

@max-sixty max-sixty removed the needs triage Issue that has not been reviewed by xarray team member label Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants