empty attributes silently change

[malmans2]

What happened?

When attrs are not specified in a new Dataset/DataArray, they're changed to {} whenever they are used.
Because of that, an object can slightly change silently.

What did you expect to happen?

In the example below, the tokens should be identical.

Minimal Complete Verifiable Example

import xarray as xr
import dask

ds = xr.Dataset({"foo": [0]})  # the assertion below would be OK if I specify attrs={}
token0 = dask.base.tokenize(ds)
print(ds) # this could be anything that uses attrs under the hood
token1 = dask.base.tokenize(ds)

assert token0 == token1
# AssertionError:

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.

Relevant log output

No response

Anything else we need to know?

I thought we could store {} since the very beginning, adding the following here:

        if attrs is None:
            attrs = {}

But a few tests failed, so it's probably the wrong way to fix this:

_______________________________________________________________________________________________________________________ TestDask.test_attrs_mfdataset ________________________________________________________________________________________________________________________

self = <xarray.tests.test_backends.TestDask object at 0x151cf2ef0>

    def test_attrs_mfdataset(self) -> None:
        original = Dataset({"foo": ("x", np.random.randn(10))})
        with create_tmp_file() as tmp1:
            with create_tmp_file() as tmp2:
                ds1 = original.isel(x=slice(5))
                ds2 = original.isel(x=slice(5, 10))
                ds1.attrs["test1"] = "foo"
                ds2.attrs["test2"] = "bar"
                ds1.to_netcdf(tmp1)
                ds2.to_netcdf(tmp2)
                with open_mfdataset(
                    [tmp1, tmp2], concat_dim="x", combine="nested"
                ) as actual:
                    # presumes that attributes inherited from
                    # first dataset loaded
                    assert actual.test1 == ds1.test1
                    # attributes from ds2 are not retained, e.g.,
>                   with pytest.raises(AttributeError, match=r"no attribute"):
E                   Failed: DID NOT RAISE <class 'AttributeError'>

/Users/mattia/MyGit/xarray/xarray/tests/test_backends.py:3576: Failed
__________________________________________________________________________________________________________________ TestDask.test_open_mfdataset_attrs_file ___________________________________________________________________________________________________________________

self = <xarray.tests.test_backends.TestDask object at 0x151cf1de0>

    def test_open_mfdataset_attrs_file(self) -> None:
        original = Dataset({"foo": ("x", np.random.randn(10))})
        with create_tmp_files(2) as (tmp1, tmp2):
            ds1 = original.isel(x=slice(5))
            ds2 = original.isel(x=slice(5, 10))
            ds1.attrs["test1"] = "foo"
            ds2.attrs["test2"] = "bar"
            ds1.to_netcdf(tmp1)
            ds2.to_netcdf(tmp2)
            with open_mfdataset(
                [tmp1, tmp2], concat_dim="x", combine="nested", attrs_file=tmp2
            ) as actual:
                # attributes are inherited from the master file
                assert actual.attrs["test2"] == ds2.attrs["test2"]
                # attributes from ds1 are not retained, e.g.,
>               assert "test1" not in actual.attrs
E               AssertionError: assert 'test1' not in {'test1': 'foo', 'test2': 'bar'}
E                +  where {'test1': 'foo', 'test2': 'bar'} = <xarray.Dataset>\nDimensions:  (x: 10)\nDimensions without coordinates: x\nData variables:\n    foo      (x) float64 dask.array<chunksize=(5,), meta=np.ndarray>\nAttributes:\n    test1:    foo\n    test2:    bar.attrs

/Users/mattia/MyGit/xarray/xarray/tests/test_backends.py:3594: AssertionError
________________________________________________________________________________________________________________ TestDask.test_open_mfdataset_attrs_file_path ________________________________________________________________________________________________________________

self = <xarray.tests.test_backends.TestDask object at 0x151cf1c30>

    def test_open_mfdataset_attrs_file_path(self) -> None:
        original = Dataset({"foo": ("x", np.random.randn(10))})
        with create_tmp_files(2) as (tmp1, tmp2):
            tmp1 = Path(tmp1)
            tmp2 = Path(tmp2)
            ds1 = original.isel(x=slice(5))
            ds2 = original.isel(x=slice(5, 10))
            ds1.attrs["test1"] = "foo"
            ds2.attrs["test2"] = "bar"
            ds1.to_netcdf(tmp1)
            ds2.to_netcdf(tmp2)
            with open_mfdataset(
                [tmp1, tmp2], concat_dim="x", combine="nested", attrs_file=tmp2
            ) as actual:
                # attributes are inherited from the master file
                assert actual.attrs["test2"] == ds2.attrs["test2"]
                # attributes from ds1 are not retained, e.g.,
>               assert "test1" not in actual.attrs
E               AssertionError: assert 'test1' not in {'test1': 'foo', 'test2': 'bar'}
E                +  where {'test1': 'foo', 'test2': 'bar'} = <xarray.Dataset>\nDimensions:  (x: 10)\nDimensions without coordinates: x\nData variables:\n    foo      (x) float64 dask.array<chunksize=(5,), meta=np.ndarray>\nAttributes:\n    test1:    foo\n    test2:    bar.attrs

/Users/mattia/MyGit/xarray/xarray/tests/test_backends.py:3613: AssertionError
============================================================================================================================== warnings summary ==============================================================================================================================
../../miniconda3/envs/xarray/lib/python3.10/site-packages/seaborn/rcmod.py:82
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/seaborn/rcmod.py:82: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
    if LooseVersion(mpl.__version__) >= "3.0":

../../miniconda3/envs/xarray/lib/python3.10/site-packages/setuptools/_distutils/version.py:346
xarray/tests/test_backends.py::TestNetCDF4Data::test_zero_dimensional_variable
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/setuptools/_distutils/version.py:346: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
    other = LooseVersion(other)

xarray/tests/test_array_api.py:10
  /Users/mattia/MyGit/xarray/xarray/tests/test_array_api.py:10: UserWarning: The numpy.array_api submodule is still experimental. See NEP 47.
    import numpy.array_api as xp  # isort:skip

xarray/tests/test_accessor_dt.py: 7 warnings
xarray/tests/test_cftime_offsets.py: 5 warnings
xarray/tests/test_cftimeindex.py: 64 warnings
xarray/tests/test_cftimeindex_resample.py: 488 warnings
xarray/tests/test_missing.py: 2 warnings
  /Users/mattia/MyGit/xarray/xarray/coding/times.py:365: FutureWarning: Index.ravel returning ndarray is deprecated; in a future version this will return a view on self.
    sample = dates.ravel()[0]

xarray/tests/test_backends.py::TestNetCDF4Data::test_zero_dimensional_variable
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/cfgrib/xarray_plugin.py:11: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
    if LooseVersion(xr.__version__) <= "0.17.0":

xarray/tests/test_backends.py::TestDask::test_inline_array
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/_pytest/python.py:192: RuntimeWarning: deallocating CachingFileManager(<class 'netCDF4._netCDF4.Dataset'>, '/var/folders/_x/gdn6kyqn5d5g9j_ygdpcv5vm0000gp/T/tmp7ww27uxi/temp-2317.nc', mode='r', kwargs={'clobber': True, 'diskless': False, 'persist': False, 'format': 'NETCDF4'}), but file is not already closed. This may indicate a bug.
    result = testfunction(**testargs)

xarray/tests/test_backends.py::test_open_fsspec
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/fsspec/implementations/cached.py:589: ResourceWarning: unclosed file <_io.BufferedReader name='/var/folders/_x/gdn6kyqn5d5g9j_ygdpcv5vm0000gp/T/tmp8f034evp/0d56871fd8c14f69c81fd11bcd488de08bd1efce70691c6143d2a5f88be9ca84'>
    out[p] = open(fn, "rb").read()
  Enable tracemalloc to get traceback where the object was allocated.
  See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.

xarray/tests/test_backends.py::test_open_fsspec
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/fsspec/implementations/cached.py:589: ResourceWarning: unclosed file <_io.BufferedReader name='/var/folders/_x/gdn6kyqn5d5g9j_ygdpcv5vm0000gp/T/tmp8f034evp/836ec38b21b701a0aae052168a2a2eab45504e6c6ba441f202e77f4f79b1a7c4'>
    out[p] = open(fn, "rb").read()
  Enable tracemalloc to get traceback where the object was allocated.
  See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.

xarray/tests/test_backends.py::test_open_fsspec
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/fsspec/implementations/cached.py:589: ResourceWarning: unclosed file <_io.BufferedReader name='/var/folders/_x/gdn6kyqn5d5g9j_ygdpcv5vm0000gp/T/tmpmf5ddc2u/5b50e5f9d1df25a3c114c62d7a9dcfcd80615885572d4e2cb48894b48a393262'>
    out[p] = open(fn, "rb").read()
  Enable tracemalloc to get traceback where the object was allocated.
  See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.

xarray/tests/test_backends.py::test_open_fsspec
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/fsspec/implementations/cached.py:589: ResourceWarning: unclosed file <_io.BufferedReader name='/var/folders/_x/gdn6kyqn5d5g9j_ygdpcv5vm0000gp/T/tmpmf5ddc2u/0d56871fd8c14f69c81fd11bcd488de08bd1efce70691c6143d2a5f88be9ca84'>
    out[p] = open(fn, "rb").read()
  Enable tracemalloc to get traceback where the object was allocated.
  See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.

xarray/tests/test_backends.py::test_open_fsspec
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/fsspec/implementations/cached.py:589: ResourceWarning: unclosed file <_io.BufferedReader name='/var/folders/_x/gdn6kyqn5d5g9j_ygdpcv5vm0000gp/T/tmpmf5ddc2u/77aeffecc910b7c6882406131a4d36469d935a55c401298dbce90adb89d7d275'>
    out[p] = open(fn, "rb").read()
  Enable tracemalloc to get traceback where the object was allocated.
  See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.

xarray/tests/test_backends.py::test_open_fsspec
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/fsspec/implementations/cached.py:589: ResourceWarning: unclosed file <_io.BufferedReader name='/var/folders/_x/gdn6kyqn5d5g9j_ygdpcv5vm0000gp/T/tmpmf5ddc2u/836ec38b21b701a0aae052168a2a2eab45504e6c6ba441f202e77f4f79b1a7c4'>
    out[p] = open(fn, "rb").read()
  Enable tracemalloc to get traceback where the object was allocated.
  See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.

xarray/tests/test_calendar_ops.py: 14 warnings
xarray/tests/test_cftime_offsets.py: 12 warnings
xarray/tests/test_computation.py: 4 warnings
  /Users/mattia/MyGit/xarray/xarray/coding/cftime_offsets.py:1130: FutureWarning: Argument `closed` is deprecated in favor of `inclusive`.
    return pd.date_range(

xarray/tests/test_dataarray.py::TestDataArray::test_to_and_from_cdms2_sgrid
xarray/tests/test_dataarray.py::TestDataArray::test_to_and_from_cdms2_sgrid
xarray/tests/test_dataarray.py::TestDataArray::test_to_and_from_cdms2_sgrid
xarray/tests/test_dataarray.py::TestDataArray::test_to_and_from_cdms2_sgrid
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/numpy/ma/core.py:7891: DeprecationWarning: elementwise comparison failed; this will raise an error in the future.
    if not np.all(xinf == filled(np.isinf(y), False)):

xarray/tests/test_dataset.py: 12 warnings
xarray/tests/test_units.py: 20 warnings
  /Users/mattia/MyGit/xarray/xarray/core/common.py:1079: PendingDeprecationWarning: dropping variables using `drop` will be deprecated; using drop_vars is encouraged.
    cond_wdim = cond.drop(var for var in cond if dim not in cond[var].dims)

xarray/tests/test_distributed.py::test_open_mfdataset_can_open_files_with_cftime_index
xarray/tests/test_distributed.py::test_open_mfdataset_can_open_files_with_cftime_index
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/tornado/ioloop.py:263: DeprecationWarning: There is no current event loop
    loop = asyncio.get_event_loop()

xarray/tests/test_distributed.py::test_open_mfdataset_can_open_files_with_cftime_index
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/tornado/platform/asyncio.py:326: DeprecationWarning: There is no current event loop
    self.old_asyncio = asyncio.get_event_loop()

xarray/tests/test_distributed.py::test_open_mfdataset_can_open_files_with_cftime_index
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/tornado/platform/asyncio.py:193: DeprecationWarning: There is no current event loop
    old_loop = asyncio.get_event_loop()

xarray/tests/test_duck_array_ops.py::test_datetime_mean[True]
xarray/tests/test_duck_array_ops.py::test_datetime_mean[True]
xarray/tests/test_duck_array_ops.py::test_datetime_mean[True]
xarray/tests/test_duck_array_ops.py::test_datetime_mean[True]
xarray/tests/test_duck_array_ops.py::test_datetime_mean[True]
xarray/tests/test_duck_array_ops.py::test_datetime_mean[True]
xarray/tests/test_duck_array_ops.py::test_datetime_mean[True]
xarray/tests/test_duck_array_ops.py::test_datetime_mean[True]
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/dask/array/reductions.py:611: RuntimeWarning: All-NaN slice encountered
    return np.nanmin(x_chunk, axis=axis, keepdims=keepdims)

xarray/tests/test_duck_array_ops.py::test_datetime_mean[True]
xarray/tests/test_duck_array_ops.py::test_datetime_mean[True]
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/dask/array/reductions.py:611: RuntimeWarning: All-NaN axis encountered
    return np.nanmin(x_chunk, axis=axis, keepdims=keepdims)

xarray/tests/test_groupby.py::test_groupby_drops_nans
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/flox/aggregate_flox.py:105: RuntimeWarning: invalid value encountered in true_divide
    out /= nanlen(group_idx, array, size=size, axis=axis, fill_value=0)

xarray/tests/test_plot.py::TestFacetGrid::test_facetgrid_polar
xarray/tests/test_plot.py::TestFacetGrid::test_facetgrid_polar
xarray/tests/test_plot.py::TestFacetGrid::test_facetgrid_polar
  /Users/mattia/MyGit/xarray/xarray/plot/plot.py:1478: MatplotlibDeprecationWarning: Auto-removal of grids by pcolor() and pcolormesh() is deprecated since 3.5 and will be removed two minor releases later; please call grid(False) first.
    primitive = ax.pcolormesh(x, y, z, **kwargs)

xarray/tests/test_print_versions.py::test_show_versions
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/_distutils_hack/__init__.py:33: UserWarning: Setuptools is replacing distutils.
    warnings.warn("Setuptools is replacing distutils.")

xarray/tests/test_variable.py::TestVariableWithDask::test_eq_all_dtypes
xarray/tests/test_variable.py::TestVariableWithDask::test_eq_all_dtypes
xarray/tests/test_variable.py::TestVariableWithDask::test_eq_all_dtypes
xarray/tests/test_variable.py::TestVariableWithDask::test_eq_all_dtypes
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/dask/core.py:119: FutureWarning: elementwise comparison failed; returning scalar instead, but in the future will perform elementwise comparison
    return func(*(_execute_task(a, cache) for a in args))

xarray/tests/test_weighted.py::test_weighted_quantile_equal_weights[1-True-0.5-da2]
xarray/tests/test_weighted.py::test_weighted_quantile_equal_weights[1-True-q1-da2]
xarray/tests/test_weighted.py::test_weighted_quantile_equal_weights[3.14-True-0.5-da2]
xarray/tests/test_weighted.py::test_weighted_quantile_equal_weights[3.14-True-q1-da2]
  /Users/mattia/miniconda3/envs/xarray/lib/python3.10/site-packages/numpy/lib/nanfunctions.py:1560: RuntimeWarning: All-NaN slice encountered
    r, k = function_base._ureduce(a,

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
========================================================================================================================== short test summary info ===========================================================================================================================
FAILED xarray/tests/test_backends.py::TestDask::test_attrs_mfdataset - Failed: DID NOT RAISE <class 'AttributeError'>
FAILED xarray/tests/test_backends.py::TestDask::test_open_mfdataset_attrs_file - AssertionError: assert 'test1' not in {'test1': 'foo', 'test2': 'bar'}
FAILED xarray/tests/test_backends.py::TestDask::test_open_mfdataset_attrs_file_path - AssertionError: assert 'test1' not in {'test1': 'foo', 'test2': 'bar'}
===================================================================================== 3 failed, 14484 passed, 1189 skipped, 211 xfailed, 65 xpassed, 671 warnings in 1223.86s (0:20:23) ======================================================================================
/Users/mattia/miniconda3/envs/xarray/lib/python3.10/multiprocessing/resource_tracker.py:224: UserWarning: resource_tracker: There appear to be 31 leaked semaphore objects to clean up at shutdown
  warnings.warn('resource_tracker: There appear to be %d '

Environment

INSTALLED VERSIONS

commit: None
python: 3.10.6 | packaged by conda-forge | (main, Aug 22 2022, 20:43:44) [Clang 13.0.1 ]
python-bits: 64
OS: Darwin
OS-release: 21.5.0
machine: x86_64
processor: i386
byteorder: little
LC_ALL: None
LANG: None
LOCALE: (None, 'UTF-8')
libhdf5: 1.12.2
libnetcdf: 4.8.1

xarray: 2022.6.0
pandas: 1.4.3
numpy: 1.23.2
scipy: None
netCDF4: 1.6.0
pydap: None
h5netcdf: None
h5py: None
Nio: None
zarr: 2.12.0
cftime: 1.6.1
nc_time_axis: None
PseudoNetCDF: None
rasterio: None
cfgrib: 0.9.10.1
iris: None
bottleneck: None
dask: 2022.8.1
distributed: 2022.8.1
matplotlib: None
cartopy: None
seaborn: None
numbagg: None
fsspec: 2022.7.1
cupy: None
pint: None
sparse: None
flox: None
numpy_groupies: None
setuptools: 65.3.0
pip: 22.2.2
conda: None
pytest: 7.1.2
IPython: 8.4.0
sphinx: 5.1.1

[dcherian]

A less invasive change would be to pass self._attrs or {} here:

xarray/xarray/core/dataset.py

Lines 746 to 751 in b018442

	def __dask_tokenize__(self):
	from dask.base import normalize_token
	return normalize_token(
	(type(self), self._variables, self._coord_names, self._attrs)
	)

(and similarly in all the other tokenize functions)

but lets see what @crusaderky thinks.

These are strictly different, but the difference should not matter for the user.

[headtr1ck]

It is not an easy fix since we do not simply want to return None, and returning an empty dict in this case without overwriting the internal one would break things like da.attrs.update(some_dict).
We would have to replace the internal _attrs already in the __init__, but not sure what else that would break.

Maybe the solution from @dcherian is the best for now.

[shoyer]

I think the bug here is the use of self._attrs in __dask_tokenize__. It should use self.attrs instead, which is basically equivalent to self._attrs or {}.