issue_comments

The inconsistency in the chunks argument is non-ideal, but that could be handled by a separate deprecation process.

There was some discussion on whether xr.open_dataset should default to chunks=None, or switch to open_zarr's chunks="auto" at https://github.com/pydata/xarray/pull/4187#issuecomment-690885702 (the last attempt at deprecating open_zarr :slightly_smiling_face:). Just bringing it up for debate.

Also, quite a few people were in favour of keeping open_zarr back in 2020 (see e.g. https://github.com/pydata/xarray/pull/4187#issuecomment-652482831), but maybe times have changed, and a consistent API is more desirable than convenience?

It loos reasonable to me. I'm not sure if the warning is needed or not - we don't expect anyone to see it, or if they do, necessarily do anything about it. It's not unusual for code interacting with a file-like object to move the file pointer.

Hmm, in that case, I'm leaning towards removing the warning. The file pointer is reset anyway after reading the magic byte number, and that hasn't caused any issues (as mentioned in https://github.com/pydata/xarray/issues/6813#issuecomment-1205503288), so it should be more or less safe. Let me push another commit. Edit: done at 929cb62977d630a00ace9747bc86066555b83d0d.

Traceback from the 2 test failures at https://github.com/pydata/xarray/actions/runs/3516849430/jobs/5893926099#step:9:252

```python-traceback =================================== FAILURES =================================== ____ TestH5NetCDFFileObject.test_open_twice ______ [gw2] linux -- Python 3.10.7 /home/runner/micromamba-root/envs/xarray-tests/bin/python

self = <xarray.tests.test_backends.TestH5NetCDFFileObject object at 0x7f211de81e40>

def test_open_twice(self) -> None:
    expected = create_test_data()
    expected.attrs["foo"] = "bar"

  with pytest.raises(ValueError, match=r"read/write pointer not at the start"):

E Failed: DID NOT RAISE <class 'ValueError'>

/home/runner/work/xarray/xarray/xarray/tests/test_backends.py:3034: Failed ___ TestH5NetCDFFileObject.test_open_fileobj _____ [gw2] linux -- Python 3.10.7 /home/runner/micromamba-root/envs/xarray-tests/bin/python

self = <xarray.tests.test_backends.TestH5NetCDFFileObject object at 0x7f211de82530>

@requires_scipy
def test_open_fileobj(self) -> None:
    # open in-memory datasets instead of local file paths
    expected = create_test_data().drop_vars("dim3")
    expected.attrs["foo"] = "bar"
    with create_tmp_file() as tmp_file:
        expected.to_netcdf(tmp_file, engine="h5netcdf")

        with open(tmp_file, "rb") as f:
            with open_dataset(f, engine="h5netcdf") as actual:
                assert_identical(expected, actual)

            f.seek(0)
            with open_dataset(f) as actual:
                assert_identical(expected, actual)

            f.seek(0)
            with BytesIO(f.read()) as bio:
                with open_dataset(bio, engine="h5netcdf") as actual:
                    assert_identical(expected, actual)

            f.seek(0)
            with pytest.raises(TypeError, match="not a valid NetCDF 3"):
                open_dataset(f, engine="scipy")

        # TODO: this additional open is required since scipy seems to close the file
        # when it fails on the TypeError (though didn't when we used
        # `raises_regex`?). Ref https://github.com/pydata/xarray/pull/5191
        with open(tmp_file, "rb") as f:
            f.seek(8)
            with pytest.raises(
                ValueError,
                match="match in any of xarray's currently installed IO",
            ):

              with pytest.warns(
                    RuntimeWarning,
                    match=re.escape("'h5netcdf' fails while guessing"),
                ):

E Failed: DID NOT WARN. No warnings of type (<class 'RuntimeWarning'>,) matching the regex were emitted. E Regex: 'h5netcdf'\ fails\ while\ guessing E Emitted warnings: [ UserWarning('cannot guess the engine, file-like object read/write pointer not at the start of the file, so resetting file pointer to zero. If this does not work, please close and reopen, or use a context manager'), E RuntimeWarning("deallocating CachingFileManager(<class 'h5netcdf.core.File'>, <_io.BufferedReader name='/tmp/tmpoxdfl12i/temp-720.nc'>, mode='r', kwargs={'invalid_netcdf': None, 'decode_vlen_strings': True}, manager_id='b62ec6c8-b328-409c-bc5d-bbab265bea51'), but file is not already closed. This may indicate a bug.")]

/home/runner/work/xarray/xarray/xarray/tests/test_backends.py:3076: Failed =========================== short test summary info ============================ FAILED xarray/tests/test_backends.py::TestH5NetCDFFileObject::test_open_twice - Failed: DID NOT RAISE <class 'ValueError'> FAILED xarray/tests/test_backends.py::TestH5NetCDFFileObject::test_open_fileobj - Failed: DID NOT WARN. No warnings of type (<class 'RuntimeWarning'>,) matching the regex were emitted. Regex: 'h5netcdf'\ fails\ while\ guessing Emitted warnings: [ UserWarning('cannot guess the engine, file-like object read/write pointer not at the start of the file, so resetting file pointer to zero. If this does not work, please close and reopen, or use a context manager'), RuntimeWarning("deallocating CachingFileManager(<class 'h5netcdf.core.File'>, <_io.BufferedReader name='/tmp/tmpoxdfl12i/temp-720.nc'>, mode='r', kwargs={'invalid_netcdf': None, 'decode_vlen_strings': True}, manager_id='b62ec6c8-b328-409c-bc5d-bbab265bea51'), but file is not already closed. This may indicate a bug.")] = 2 failed, 14608 passed, 1190 skipped, 203 xfailed, 73 xpassed, 54 warnings in 581.98s (0:09:41) = ```

Just hitting into this same issue mentioned downstream at https://github.com/xarray-contrib/datatree/pull/130 while trying to read ICESat-2 HDF5 files from S3, but realized that the fix should happening in xarray, so I've started a PR at #7304 to fix this (thanks @djhoese for the code snippet at https://github.com/pydata/xarray/issues/6813#issuecomment-1204300671)! Will look into the unit test failures and fix them one by one as they pop up on CI.

I'm getting a similar issue with xarray=0.20.1 when rioxarray and rasterio are both installed, looks like #5931 didn't fully fix things? Here's the full traceback.

```python-traceback __ testopen_variable_filter[open_rasterio_engine] ___

open_rasterio = <function open_rasterio_engine at 0x7fa99ca7ec10>

def test_open_variable_filter(open_rasterio):

  with open_rasterio(
        os.path.join(TEST_INPUT_DATA_DIR, "PLANET_SCOPE_3D.nc"), variable=["blue"]
    ) as rds:

test/integration/test_integration__io.py:185:

test/conftest.py:103: in open_rasterio_engine return xr.open_dataset(file_name_or_object, engine="rasterio", **kwargs) ../../../miniconda3/envs/rioxarray/lib/python3.9/site-packages/xarray/backends/api.py:481: in open_dataset backend = plugins.get_backend(engine) ../../../miniconda3/envs/rioxarray/lib/python3.9/site-packages/xarray/backends/plugins.py:158: in get_backend engines = list_engines() ../../../miniconda3/envs/rioxarray/lib/python3.9/site-packages/xarray/backends/plugins.py:103: in list_engines return build_engines(entrypoints) ../../../miniconda3/envs/rioxarray/lib/python3.9/site-packages/xarray/backends/plugins.py:92: in build_engines entrypoints = remove_duplicates(entrypoints)

entrypoints = [EntryPoint(name='rasterio', value='rioxarray.xarray_plugin:RasterioBackend', group='xarray.backends'), EntryPoint(nam...rray.backends'), EntryPoint(name='rasterio', value='rioxarray.xarray_plugin:RasterioBackend', group='xarray.backends')]

def remove_duplicates(entrypoints):
    # sort and group entrypoints by name
    entrypoints = sorted(entrypoints, key=lambda ep: ep.name)
    entrypoints_grouped = itertools.groupby(entrypoints, key=lambda ep: ep.name)
    # check if there are multiple entrypoints for the same name
    unique_entrypoints = []
    for name, matches in entrypoints_grouped:
        matches = list(matches)
        unique_entrypoints.append(matches[0])
        matches_len = len(matches)
        if matches_len > 1:

          selected_module_name = matches[0].module_name

E AttributeError: 'EntryPoint' object has no attribute 'module_name'

../../../miniconda3/envs/rioxarray/lib/python3.9/site-packages/xarray/backends/plugins.py:29: AttributeError ================================ warnings summary ================================= test/integration/test_integration__io.py::test_open_variable_filter[open_rasterio] /home/username/projects/rioxarray/rioxarray/_io.py:366: DeprecationWarning: string or file could not be read to its end due to unmatched data; this will raise a ValueError in the future. new_val = np.fromstring(value.strip("{}"), dtype="float", sep=",")

-- Docs: https://docs.pytest.org/en/stable/warnings.html ============================= short test summary info ============================= FAILED test/integration/test_integration__io.py::test_open_variable_filter[open_rasterio_engine] !!!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!

```

Output of xr.show_versions()

Just a general comment on the xr.open_dataset(engine="zarr") part, I prefer to keep or reduce the amount of chunks= options (i.e. Option 1) rather than add another chunks="encoded" option.

For those who are confused, this is the current state of xr.open_mfdataset (correct me if I'm wrong):

| :arrow_down: engine\chunk :arrow_right: | None (default) | 'auto' | {} | -1 | |--------------------------------------------------------| -------------------|-------|----|-------| | None (i.e. default for NetCDF) | np.ndarray | dask.Array (produces origintal chunks as in NetCDF obj??) | dask.Array (rechunked into 1 chunk) | dask.Array (rechunked into 1 chunk) | | zarr | np.ndarray | dask.Array (original chunks as in Zarr obj) | dask.Array (original chunks as in Zarr obj) | dask.Array (rechunked into 1 chunk + UserWarning) |

Sample code to test (run in jupyter notebook to see the dask chunk visual):

I agree. I think we should keep open_zarr around.

Just wanted to mention that two of the reviewers in the last PR (see https://github.com/pydata/xarray/pull/4003#issuecomment-619644606 and https://github.com/pydata/xarray/pull/4003#issuecomment-620169860) seemed in favour of deprecating open_zarr. If I'm counting the votes correctly (did I miss anyone?), that's 2 for, and 2 against. We'll need a tiebreaker :laughing:

As reminder (because it took me a while to remember!), one goal with this refactor is to have open_mfdataset work with all backends (including zarr and rasterio) by specifying the engine kwarg.

Yes exactly, time does fly (half a year has gone by already!).

Currently I'm trying to piggyback Zarr into test_openmfdataset_manyfiles from #1983, ~~and am currently having trouble finding out why opening Zarr stores via open_mfdataset doesn't return a dask backed array like the other engines (Edit: it only happens when chunks is None, see https://github.com/pydata/xarray/pull/4187#discussion_r448734418). Might need to spend another day digging through the code to see if this is expected behaviour.~~ Edit: got a workaround solution in b3d6a6a46f8ead25b6f7f593f7b46f43a4de650c by using chunks="auto" as was the default in open_zarr.

As a note I'm working on implementing zarr spec v3 in zarr-python, still deciding how we want to handle the new spec/API.

If there are any changes that you would like or dislike in an API, feedback is welcome.

Thanks for chipping in @Carreau! I'm sure the community will have some useful suggestions. Just cross-referencing https://zarr-developers.github.io/zarr/specs/2019/06/19/zarr-v3-update.html so others can get a better feel for where things are at.

I wonder if it's really worth deprecating open_zarr(). open_dataset(..., engine='zarr') is a bit more verbose, especially with backend_kwargs to pass optional arguments. It seems pretty harmless to keep open_zarr() around, especially if it's just an alias for open_datraset(engine='zarr').

Depends on which line in the Zen of Python you want to follow - "Simple is better than complex", or "There should be one-- and preferably only one --obvious way to do it". From a maintenance perspective, it's balancing the cost of a deprecation cycle vs writing code that tests both instances I guess.

We could also automatically detect zarr stores in open_dataset without requiring engine='zarr' if:

1. the argument inherits from collections.abc.Mapping, and
2. it contains a key '.zgroup', corresponding to zarr metadata.

As for the annoyance of needing to write backend_kwargs={"consolidated": True}, I wonder if we could detect this automatically by checking for the existence of a .zmetadata key? This would add a small amount of overhead (one file access) but this probably would not be prohibitively expensive.

These are some pretty good ideas. I also wonder if there's a way to mimic the dataset identifiers like in rasterio, something like xr.open_dataset("zarr:some_zarrfile.zarr"). Feels a lot more like fsspec's url chaining too.

Counter-argument would be that the cyclomatic complexity of open_dataset is already too high, and it really should be refactored before adding more 'magic'. Especially if new backend engines come online (e.g. #4142).

Four more failures, something to do with dask? Seems related to #3919 and #3921.

• [ ] TestZarrDictStore.test_vectorized_indexing - IndexError: only slices with step >= 1 are supported
• [x] TestZarrDictStore.test_manual_chunk - ZeroDivisionError: integer division or modulo by zero
• [ ] TestZarrDirectoryStore.test_vectorized_indexing - IndexError: only slices with step >= 1 are supported
• [x] TestZarrDirectoryStore.test_manual_chunk - ZeroDivisionError: integer division or modulo by zero

Edit: Fixed the ZeroDivisionErrror in 6fbeadf41a1a547383da0c8f4499c99099dbdf97. The IndexError was fixed in a hacky way though, see https://github.com/pydata/xarray/pull/4187#discussion_r448077275.

Is it ok to drop the deprecated auto_chunk tests here in this PR (or leave it to another PR)? The deprecation warning was first added in https://github.com/pydata/xarray/pull/2530/commits/ae4cf0ab19b3e563bde90a48b3e6ee615930d4a1, and I see that auto_chunk was used back in v0.12.1 at http://xarray.pydata.org/en/v0.12.1/generated/xarray.open_zarr.html.

Nevermind, I found it. There was an if that should have been an elif. Onward to the next error - UnboundLocalError. Edit: Also fixed!

This is the one test failure (AttributeError) on Linux py36-bare-minimum:

Been scratching my head debugging this one. There doesn't seem to be an obvious reason why this test is failing, since 1) this test isn't for Zarr and 2) this test shouldn't be affected by the new if blocks checking if engine=="zarr". Will need to double check the logic here.

Sure, I can move it, but I just wanted to make sure @Mikejmnez gets the credit for this PR. Edit: moved to https://github.com/pydata/xarray/pull/4187.

@Mikejmnez, do you mind if I pick up working on this branch? I'd be really keen to see it get into xarray 0.16, and then it will be possible to resolve the intake-xarray issue at https://github.com/intake/intake-xarray/issues/70. ~~Not sure if it's possible to get commit access here, or if I should just submit a PR to your fork, or maybe there's a better way?~~ Edit: I've opened up a pull request to the fork.

Cool, there doesn't seem to be an easy [theme=dark] solution (at least from my poor CSS knowledge), so I've posted a question on the Atom forums at https://discuss.atom.io/t/how-to-detect-dark-theme-on-atom/74937 to ask.

This looks awesome! Is it possible to port this to the Atom editor as well? This is what it looks like currently on the 'One Dark' Atom editor theme:

xr.show_versions() gets its libnetcdf version from netCDF4 (specially netCDF4.__netcdf4libversion__). So I'm guessing that somehow netCDF4 is picking up libnetcdf from somewhere else -- maybe you pip installed it? It might be worth trying another fresh conda environment...

I'm not sure where it's picking up the libnetcdf 4.6.3 version from, but I found your comment at https://github.com/pydata/xarray/issues/2535#issuecomment-445944261 and think it might indeed be an incompatibility issue with rasterio and netCDF4 binary wheels (do rasterio wheels include netcdf binaries?). Probably somewhat related to https://github.com/mapbox/rasterio/issues/1574 too.

Managed to get things to work by combining the workaround in this Pull Request and StackOverflow post, basically having pip compile the netcdf python package from source instead of using the wheel:

bash HDF5_DIR=$CONDA_PREFIX pip install --no-binary netCDF4 netCDF4==1.4.2 where $CONDA_PREFIX is the path to the conda environment e.g. /home/jovyan/.conda/envs/name-of-env. I've tested my MCVE code sample above and it works up to the latest netCDF4==1.5.1.2 version!

Yes, there's https://gdal.org/drivers/raster/netcdf.html :smile: I've done a bit more debugging (having temporarily isolated salem from my script) and am still having issues with my setup.

The clean xarray-tests conda environment that works with netcdf==1.5.1.2 has libnetcdf: 4.6.2, but for some strange reason, running xr.show_versions() on my setup shows libnetcdf: 4.6.3 even though conda list | grep libnetcdf shows that I've installed libnetcdf 4.6.2 h056eaf5_1002 conda-forge.

Not sure if this libnetcdf 4.6.3 version is the problem, but it stands out the most (to me at least) when looking at the diff between my setup and the clean one. Is there a way to check the order in which xarray looks for the netcdf binaries as I feel it might be a PATH related issue. Also not sure if this issue fits here in xarray or somewhere else now...

Hold on, the coordinates seems to be parsed out correctly from the netCDF file (even with netCDF==1.5.1.2) when I have a clean conda installation created following the instructions at https://xarray.pydata.org/en/latest/contributing.html#creating-a-python-environment.

I've isolated the issue and think the problem arises when I also import salem (an xarray accessor)... Will try to narrow this down before I close this issue.

Well open_rasterio did have an "experimental" warning on it in the docs :laughing:, but it was really nice having it work on GeoTIFFs and NetCDF files. I've forked the repo and will try to debug the situation a bit more. If anyone who's worked on that part of the codebase before has any pointers on what might be the cause of this issue / where to start that would be great.