Scatter vs. Lines:

Just wanted to link to a somewhat related discussion happening in brian-rose/climlab#50.

Great observation @mathause . I think there are two parts of this: - Do we want other libraries which do da.longitude to raise a mypy error? That may be a tradeoff with raising the true error on da.isel - How do we design the type hierarchy? We could add methods to DataWithCoords or add some Dataset_Or_DataArray-like type

Having methods like isel typed would be a win I think

Good point re accessors, I hadn't considered those. So sounds like raising an error on da.isel isn't possible regardless...

Following thoughts and discussions in various issues (e.g., #6836), I'd like to suggest another section to the ones in the top comment:

Deprecate pandas.MultiIndex special cases in Xarray

• remove the multi-index “dimension” coordinate (tuple elements)
• do not automatically promote pandas.MultiIndex objects as dimension + level coordinates, e.g., like in xr.Dataset(coords={“x”: pd_midx}) but instead treat it as a single duck-array.
• do not accept pandas.MultiIndex as dim argument in xarray.concat() (#7148)
• remove obj.to_index() for all xarray objects?
• (EDIT) remove Dataset.reset_index() and DataArray.reset_index()

They are source of many problems and complexities in Xarray internals (many regressions reported since the index refactor were related to those special cases) and I'm not sure that the value they add is really worth the trouble. Also, in the long term the special treatment of PandasMultiIndex vs. other Xarray multi-indexes may add some confusion.

Some of those features are widely used (e.g., the creation of Dataset / DataArray from pandas multi-indexes is used in many places in unit tests), so we would need convenient alternatives and a smooth transition.

Might be worth an issue over at numpy with the example from the test.

numpy/numpy#23886

The issue is already resolved over at numpy which is really great! It was also marked as backport. @headtr1ck How are these issues resolved currently or how do we track removing the ignore?

Might be worth an issue over at numpy with the example from the test.

https://github.com/numpy/numpy/issues/23886

Should we delete the cfgrib example instead?

https://github.com/corteva/rioxarray/issues/676

but is it not possible for it to calculate the integrated values where there were regular values?

@chfite Can you provide an example of what you would want it to do please

@vrishk Sorry for the delay here and thanks for bringing this to attention. We now have at least two requests which might move this forward (moving ensure_dtype_not_object into the backends). But this would need some discussion first, how to do this.

@mktippett Thanks for raising this. The issue should be cleared after #7888 is merged.

You could use DataArray.round to round to significant decimals.

Thank you @keewis

I think the only other one is dask, which should also work.

This seems like a real easy fix? axis = tuple(self.get_axis_num(d) for d in dim)

EDIT: the Array API seems to type axis as Optional[Union[int, Tuple[int, ...]]] pretty consistently, so it seems like we should always pass tuples down to the array library

xarray/tests/test_coding_strings.py:36: error: No overload variant of "dtype" matches argument types "str", "Dict[str, Type[str]]" [call-overload]

cc @Illviljan @headtr1ck

Given that this error only is caused when Python is shutting down, which is exactly a case in which we do not need to clean up open file objects, maybe we can remove the __del__ instead?

Something like: ```python import atexit

@atexit.register def _remove_del_method(): # We don't need to close unclosed files at program exit, # and may not be able to do, because Python is cleaning up # imports. del CachingFileManager.del ```

(I have not tested this!)

that appears to work on both my laptop and my local HPC, and is arguably a lot easier to implement / understand as we don't need to make sure all the globals we use are still available (which in this case would be acquire, OPTIONS, warnings, and RuntimeWarning).

Edit: let me change the PR to do that instead

Have you verfied that this fixes things at least on your machine?

I thought I did, but apparently something changed: right now it fails because OPTIONS is not available anymore, so we might have to add a reference to that, as well. Additionally, for the warning we use warnings.warn and RuntimeWarning that might also have disappeared already (but those are standard library / builtins, so hopefully not?)

In any case, you can verify this, too: - create a new environment using mamba create -n test python=3.11 numpy pandas packaging pooch netcdf4 and activate it - run pip install 'dask[array]' to install dask without distributed (this appears to make a difference for me, not sure if that's the same elsewhere) - editable-install xarray so we can easily switch between branches - run python -c 'import xarray as xr; ds = xr.tutorial.open_dataset("air_temperature", chunks={})'

This should print an error for main and shouldn't with this branch (confirmed on my local HPC).

the issue is that this doesn't occur on normal garbage collection but only on interpreter shutdown. So really, I don't think we have any way to test this using pytest as that itself is written in python (unless of course we can make use of sub-interpreters, but that might be more trouble than it's worth).

I agree that this seems very hard to test!

Have you verfied that this fixes things at least on your machine?

Hmmm ndim is in the array api so potentially we could just update the test.

Please ask over at the cfgrib repo. But it does look like a bad environment / bad install.

@tomwhite I've added tests to check the backend code for vlen string dtype metadadata. Also had to add specific check for the h5py vlen string metadata. I think we've covered everything for the proposed change to allow empty vlen strings dtype metadata.

I'm looking at the mypy error and do not have the slightest clue what and where to change. Any help appreciated.

this is #7879 (and thus probably #7079). I suspect our locks are not working properly, but in any case we really should try to fix this.

No module named '_cffi_backend'

Does simply import cfgrib work for you? I suspect it doesn't, which would explain the issue. It's unfortunate that the error is rewritten to "unknown engine", but I'm not sure how we would detect that it's a dependency that fails.

Thanks @mgunyho !

No, this should still work: sh conda create -n test -c conda-forge xarray ipython python=3.11 cfgrib pooch conda activate test ipython ```python import xarray as xr

xr.tutorial.open_dataset("era5-2mt-2019-03-uk.grib") `` We somewhat recently dropped the builtincfgribengine in favor of the one provided by thecfgribpackage (and that is also the reason why the example in the docs fails:cfgrib` is not installed into the docs environment anymore, which is definitely an oversight).

Which version of cfgrib do you have? For reference, in the environment built with the command above I have cfgrib=0.9.10.3

great, thanks for the confirmation!

as stated by the exception, the cfgrib engine is unknown, which usually means you're missing the cfgrib package (or this is a environment issue). If you did indeed install it, can you post the output of either conda list or pip list (in case you're not using conda)?

I am closing this. Feel free to re-open/ or open a new issue.

I would have thought that the global (or module level here) variable/function aquire should have at least one reference until after the deletion of the object.

I think this is intended (though certainly not very easy to get right): see the second part of the warning in the __del__ documentation.

If I'm reading the different issues correctly, that means this is a duplicate of #7079

closing, since anything still missing should be feature requests for xncml

I think this has been fixed by xncml and/or kerchunk.

One other tricky edge case that occurs to me is one where an extreme fill value (e.g. 1e30) is used for floating point fields. If we decode the times first, it might appear that the dates cannot be represented as nanosecond-precision values, but in reality they would be. We may need to think more about how to handle this edge case in addition to #7817.

Actually, it goes away with pip install jinja2. We don't use jinja2 at all, so either this is some kind of weird effect on garbage collection (a timing issue?), or dask is doing something differently as soon as jinja2 is available.

Edit: most likely this is a timing issue... the offending line tries to make use of the internal acquire function, which I think at that point has already been destroyed. To fix that, I think we need to somehow store a reference on the file manager?

I can reproduce this locally: - download and unpack the files from https://github.com/pydata/xarray/issues/7814#issuecomment-1535168128 - use mamba create -n test python=3.11 xarray netcdf4 to create the environment (note: no dask) - use pip install "dask[array]" to install dask (does not pull distributed like the package from conda-forge) - put the code into a script and execute it

For reference, the full traceback is: pytb Exception ignored in: <function CachingFileManager.__del__ at 0x7fbdb237b430> Traceback (most recent call last): File "/home/jmagin/.local/opt/mambaforge/envs/test/lib/python3.9/site-packages/xarray/backends/file_manager.py", line 246, in __del__ TypeError: 'NoneType' object is not callable

As far as I can tell, this means we're using something from distributed with a broken fallback, since the error goes away as soon as I install distributed.

cos is a float operation so I would lean towards using a isclose-check: xr.testing.assert_allclose(a + 1, np.cos(a)).

how did you set up your environment? This works for me: sh mamba create -n test python=3.11 xarray dask netcdf4 pooch ipython mamba activate test ipython python xr.tutorial.open_dataset("rasm", chunks={})

Interestingly enough, though, is that you should only see this with xarray=2023.5.0, while your environment claims to have xarray=2023.4.2. It seems there is something wrong with your environment?

Yes float64 should cause less imprecision. You can convert using astype:

```python import numpy as np import xarray as xr

da = xr.DataArray(np.array([1, 2], dtype=np.float32))

da = da.astype(float) ```

As for the other problems I think you are better of asking the people over at rioxarray. However, you should first gather all the steps you did to convert the data as code. This way it is easier to see what you are actually doing.

I agree, this change should be fine.

xarray handles nan values and ignores them per default - so you don't need to remove them. For example: ```python import numpy as np import xarray as xr

da = xr.DataArray([1, 2, 3, np.nan]) da.mean() `` If you have precision problems - that might be because you havefloat32` values.

I don't know what goes wrong with your lon values - that is an issue in the reprojection. You could convert them to 0...360 by using

```python

lon_dim = "x" new_lon = np.mod(da[lon_dim], 360) da = da.assign_coords({lon_dim: new_lon}) da.reindex({lon_dim : np.sort(da[lon_dim])})

```

Thanks @ghiggi for your comment.

The problem is we have at least two contradicting user requests here, see #7328 and #7862.

I'm sure there is a solution to accommodate both sides.

Thanks for raising this @vhaasteren ! We want to do what we can to support users from all fields of science :)

I would be okay with that change (especially as it's not really special-casing pint-pulsar, so much as generalizing an existing error-catching mechanism), but would defer to the opinion of @keewis on this.

Solution for those who just found this issue:

Just re-install xarray. pip install -e . is sufficient. Re-installing any way through pip/conda should register the dask chunkmanager entrypoint.

@Illviljan I brought this up in the xarray team call today and we decided that since this only affects people who have previously cloned the xarray repository, are using a development install, and then updated by pulling changes from main; this problem only affects maybe ~10-20 people worldwide, all of whom are developers who are equipped to quickly solve it.

I'm going to add a note into the what's new entry for this version now - if you think we need to do more then let me know.

EDIT: I added a note to whatsnew in https://github.com/pydata/xarray/commit/69445c62953958488a6b35fafd8b9cfd6c0374a5, and updated the release notes.

We dropped Python 3.8 support prior to the Pandas 2 release and have no plans to backport support at this time.

xref: #7765

Thanks for trying. I can't think of any downsides for the netcdf4-fix, as it just adds the needed metadata to the object-dtype. But you never know, so it would be good to get another set of eyes on it.

So it looks like the changes here with the fix in my branch will get your issue resolved @tomwhite, right?

I'm a bit worried, that this might break other users workflows, if they depend on the current conversion to floating point for some reason. Also other backends might rely on this feature. Especially because this has been there since the early days when xarray was known as xray.

@dcherian What would be the way to go here?

There is also a somehow contradicting issue in #7868.

My main question here is, why is dask not trying to retrieve the object types from dtype.metadata? Or does it and fail for some reason?.

@tomwhite I've put a commit with changes to zarr/netcdf4-backends which should preserve the dtype metadata here: https://github.com/kmuehlbauer/xarray/tree/preserve-vlen-string-dtype.

I'm not really sure if that is the right location, but as it was already present that location at netcdf4-backend I think it will do.

Do you know where the in-place modification is happening? We could just copy there and fix this particular issue.

@tomwhite Special casing on netcdf4 backend should be possible, too.

But it might need fixing at zarr backend, too:

python ds = xr.Dataset({"a": np.array([], dtype=xr.coding.strings.create_vlen_dtype(str))}) print(f"dtype: {ds['a'].dtype}") print(f"metadata: {ds['a'].dtype.metadata}") ds.to_zarr("a.zarr") print("\n### Loading ###") with xr.open_dataset("a.zarr", engine="zarr") as ds: print(f"dtype: {ds['a'].dtype}") print(f"metadata: {ds['a'].dtype.metadata}") ```python dtype: object metadata: {'element_type': <class 'str'>}

Loading

dtype: object metadata: None ```

Could you verify the above example, please? I'm relatively new to zarr :grimacing:

Yes but there are less - so as mentioned it removes all columns/ rows with only nans, if there is at least one non-nan value the row is kept.

What is the reason that you want to get rid of the nan values?

@ghiggi Glad it works, but we still have to check if that is the correct location for the fix, as it's not CF specific.

Can you try notnull instead of isnull - I often get the boolean array wrong in where:

python da = ds['z'] da = da.where(da.notnull(), drop=True)

What is the reason that you want to get rid of the nan values?

The reason they come back is that are needed to fill the grid again. The dataframe is 1D but the dataarray is 2D.

What you can try is to use where:

python da = ds['z'] da = da.where(da.isnull(), drop=True) but it will only drop the values if the entire row/ column is nan.

Thanks @tomwhite for the PR. I've only quickly checked the approach, which looks reasonable. But those changes have implications on several locations of the backend code, which we would have to sort out.

Considering this example:

```python import numpy as np import xarray as xr print(f"creating dataset with empty string array") print("-----------------------------------------") dtype = xr.coding.strings.create_vlen_dtype(str) ds = xr.Dataset({"a": np.array([], dtype=dtype)}) print(f"dtype: {ds['a'].dtype}") print(f"metadata: {ds['a'].dtype.metadata}") ds.to_netcdf("a.nc", engine="netcdf4")

print("\nncdump") print("-------") !ncdump a.nc

engines = ["netcdf4", "h5netcdf"] for engine in engines: with xr.open_dataset("a.nc", engine=engine) as ds: print(f"\nloading with {engine}") print("-------------------") print(f"dtype: {ds['a'].dtype}") print(f"metadata: {ds['a'].dtype.metadata}") ```

```python creating dataset with empty string array

dtype: object metadata: {'element_type': <class 'str'>}

ncdump

netcdf a { dimensions: a = UNLIMITED ; // (0 currently) variables: string a(a) ; data: }

loading with netcdf4

dtype: object metadata: None

loading with h5netcdf

dtype: object metadata: {'vlen': <class 'str'>} ```

Engine netcdf4 does not roundtrip here, losing the dtype metadata information. There is special casing for h5netcdf backend, though.

The source is actually located in open_store_variable of netcdf4 backend, when the underlying data is converted to Variable (which does some object dtype twiddling).

Unfortunately I do not have an immediate solution here.

@tomwhite Sorry for the delay here. I'll respond shortly on your PR #7862, but we might have to reiterate here later

Thanks @martinfleis this is a very valuable contribution to the ecosystem!

this appears to have worked. Thanks, @martinfleis!

add the token as a ANACONDA_NIGHTLY secret.

done. Let's try it!

@ghiggi Thanks for getting this back into action. I got dragged away from the one string object issue in #7654. I'll split this out and add a PR.

@ghiggi I'd appreciate if you could test your workflows against #7869. Your example and the one over in #7652 are working AFAICT.

Do we need to build the wheel in the same way you currently do in [the TestPyPI workflow]

depends on whether we can put local versions on anaconda. PyPI and TestPyPI don't allow that, so we had to modify pyproject.toml to make setuptools-scm generate versions without the local part.

Do you have an account there so I can add you?

now I do, the username is the same as on github.

since the version with cdms2 fails on environment creation we can't really remove the special case for python=3.11 until we reached a decision on what to do with cdms2 support. I'll open a new issue for that, but otherwise this should be ready for merging.

I wonder if pure-python projects could get away with asking to install from github? This works today: sh pip install git+https://github.com/pydata/xarray.git and we have been using it downstream in nightly CI.

That said, I can see a central location being helpful, and we'd certainly be happy to add a scheduled github action to upload built packages. We'd need some help with setting that up, though, as I personally don't have any experience whatsoever in uploading to anaconda.org. Do you have documentation/examples on how to do the actual upload?

That would also allow us to stop publishing builds to TestPyPI (not sure if those are nightlies), as that seems to have accumulated quite a few packages already that due to the PyPI policy stay there forever.

cc @spencerkclark @znicholls

Great, yeah, that's a nice example without writing to disk. Indeed I saw those warnings too, but omitted them in my earlier message to focus on the encoding issue (sorry about that). I agree that these are something we should address.

The example below is only based on Variable and the cf encode/decode variable functions.

```python import xarray as xr import numpy as np

create DataArray

times = [np.datetime64("2000-01-01", "ns"), np.datetime64("NaT")] da = xr.DataArray(times, dims=["time"], name="foo") da.encoding["dtype"] = np.float64 da.encoding["_FillValue"] = 20.0

extract Variable

source_var = da.variable print("---------- source_var ------------------") print(source_var) print(source_var.encoding)

encode Variable

encoded_var = xr.conventions.encode_cf_variable(source_var) print("\n---------- encoded_var ------------------") print(encoded_var)

decode Variable

decoded_var = xr.conventions.decode_cf_variable("foo", encoded_var) print("\n---------- decoded_var ------------------") print(decoded_var.load()) ```

```python /home/kai/miniconda/envs/xarray_311/lib/python3.11/site-packages/xarray/coding/times.py:618: RuntimeWarning: invalid value encountered in cast int_num = np.asarray(num, dtype=np.int64) /home/kai/miniconda/envs/xarray_311/lib/python3.11/site-packages/xarray/coding/times.py:254: RuntimeWarning: invalid value encountered in cast flat_num_dates_ns_int = (flat_num_dates * _NS_PER_TIME_DELTA[delta]).astype( /home/kai/miniconda/envs/xarray_311/lib/python3.11/site-packages/xarray/coding/times.py:254: RuntimeWarning: invalid value encountered in cast flat_num_dates_ns_int = (flat_num_dates * _NS_PER_TIME_DELTA[delta]).astype(

---------- source_var ------------------ <xarray.Variable (time: 2)> array(['2000-01-01T00:00:00.000000000', 'NaT'], dtype='datetime64[ns]') {'dtype': <class 'numpy.float64'>, '_FillValue': 20.0} dtype num float64

---------- encoded_var ------------------ <xarray.Variable (time: 2)> array([ 0., 20.]) Attributes: units: days since 2000-01-01 00:00:00 calendar: proleptic_gregorian _FillValue: 20.0

---------- decoded_var ------------------ <xarray.Variable (time: 2)> array(['2000-01-01T00:00:00.000000000', 'NaT'], dtype='datetime64[ns]') {'_FillValue': 20.0, 'units': 'days since 2000-01-01 00:00:00', 'calendar': 'proleptic_gregorian', 'dtype': dtype('float64')} ```

@spencerkclark With current master I get the following RuntimeWarning running your code example:

• on encoding (calling to_netcdf()):

python /home/kai/miniconda/envs/xarray_311/lib/python3.11/site-packages/xarray/coding/times.py:618: RuntimeWarning: invalid value encountered in cast int_num = np.asarray(num, dtype=np.int64)

• on decoding (calling open_dataset()):

python /home/kai/miniconda/envs/xarray_311/lib/python3.11/site-packages/xarray/coding/times.py:254: RuntimeWarning: invalid value encountered in cast flat_num_dates_ns_int = (flat_num_dates * _NS_PER_TIME_DELTA[delta]).astype( /home/kai/miniconda/envs/xarray_311/lib/python3.11/site-packages/xarray/coding/times.py:254: RuntimeWarning: invalid value encountered in cast flat_num_dates_ns_int = (flat_num_dates * _NS_PER_TIME_DELTA[delta]).astype(

The latter was discussed in #7098 (casting float64 to int64), the former was aimed to be resolved with this PR.

I'll try to create a test case using Variable and the respective encoding/decoding functions without involving IO (per your suggestion @spencerkclark).

The only reason I didn't separate the chunkmanager entry points into local and other entry points was simplicity of code.

I didn't realise that might make a difference when it came to whether or not you have to pip install - I assumed that adding a new type of entry point would require re-installing no matter how I implemented it. If that's not the case perhaps we should adjust it (and re-release).

Nope, I have not tried that. I suspect things will just self heal then considering the CI without understanding the root cause.

Looking at the backends; we initialize a dict here: https://github.com/pydata/xarray/blob/d8ec3a3f6b02a8b941b484b3d254537af84b5fde/xarray/backends/common.py#L435

Stores each of our entrypoints like this: https://github.com/pydata/xarray/blob/d8ec3a3f6b02a8b941b484b3d254537af84b5fde/xarray/backends/h5netcdf_.py#L438

Then we append the local and other entrypoints together here: https://github.com/pydata/xarray/blob/d8ec3a3f6b02a8b941b484b3d254537af84b5fde/xarray/backends/plugins.py#L106-L116

But load_chunkmanagers doesn't really seem to append from a dict: https://github.com/pydata/xarray/blob/d8ec3a3f6b02a8b941b484b3d254537af84b5fde/xarray/core/parallelcompat.py#L48-L62

Why do the backends use the BACKEND_ENTRYPOINTS strategy? To avoid these cases? Or something else?

Yes, but I'm wondering what functional difference is that making here?

Have you tried doing the local pip install of the xarray dev version again? I.e. pip install -e . from the xarray folder.

The CI recreates its entire environment all the time and I don't?

```python from xarray.core.parallelcompat import list_chunkmanagers

list_chunkmanagers() Out[1]: {} ```

Hmm, it's acting as if dask is not installed/importable. Any idea what's different about your setup vs the xarray CI?

Yes daskmanager is also registered via a different entry point, but that should already be set up to happen by default.

To see which chunk managers it can find you can call

```python from xarray.core.parallelcompat import list_chunkmanagers

list_chunkmanagers() ```

I expect it will return an empty list in your case, but that's the code we should be trying to debug on your system.

before after ratio [05c7888d] [d135ab97] - 2.47±0.02s 806±6ms 0.33 pandas.ToDataFrameDask.time_to_dataframe

Our backends are stored in a dict like this: BACKEND_ENTRYPOINTS["h5netcdf"] = ("h5netcdf", H5netcdfBackendEntrypoint). Is it something similar daskmanager needs to do?

cc @TomNicholas

with #7855 and the recent change to pint we're finally down to just two test failures (and a whole lot of warnings): xarray/tests/test_dataarray.py::TestDataArray::test_to_and_from_cdms2_sgrid: ValueError: operands could not be broadcast together with shapes (3,) (4,) xarray/tests/test_ufuncs.py::test_unary: AssertionError: assert (<class 'int'> is <class 'numpy.float64'> or 1.0 == 0.9999999999999999) The first one looks like cdms2 is incompatible with a change in numpy>=1.25. Not sure if we can do anything about that, especially since there's a big warning on the cdms2 repo stating that the package is going to be retired / archived by the end of this year... I guess we should start thinking about removing cdms2 support?

The second looks like a precision issue, which we should be able to resolve by using assert_allclose instead... not sure, though, especially given numpy/numpy#23773. Otherwise we could just defer to whatever numpy is doing (of course, that assumes that full_like works properly, which might not be a good idea for a unit test): ```python @pytest.mark.parametrize( "a", [ xr.Variable(["x"], [0, 0]), xr.DataArray([0, 0], dims="x"), xr.Dataset({"y": ("x", [0, 0])}), ], ) def test_unary(a): fill_value = np.cos(0)

expected = xr.full_like(a, fill_value=fill_value, dtype=fill_value.dtype)
actual = np.cos(a)

assert_identical(actual, expected)

`` Edit: if relying onfull_like` turns out to be a concern, maybe we could use "copy + assign" instead?

Thanks @spencerkclark for taking the time. NaN has been written to disk (as you assumed). Let's have another try next week.

np.pad is an interesting example in the context of chunked arrays (xref #6807) - dask implements it (parallelized using various approaches, including map_blocks internally), but cubed currently doesn't implement it because it's not part of the array API standard. (cc @tomwhite)

Closing in favour of #7847

👏 Congrats @TomNicholas on getting this in! Such an important contribution. 👏

Woooo thanks @dcherian !

Thanks @TomNicholas Big change!

Thanks @maxhollmann I pushed a test for #2377.

I see this is your first contribution to Xarray. Welcome! #1792 would be a nice contribution if you're looking for more to do ;)

We should migrate these to rioxarray if they aren't there already.

cc @snowman2 @scottyhq @JessicaS11

@Illviljan thanks for all your comments!

Would you (or @keewis?) be willing to approve this PR now? I would really like to merge this so that I can release a version of xarray that I can use as a dependency for cubed-xarray.

Thanks @maxhollmann can you add a note to whats-new.rst please?

Think I'll stop here.

@alimanfoo, feel free to try this out if you have the time.

Results: ``` before after ratio [964d350a] [86ef9540] - 117±2ms 65.6±0.9ms 0.56 groupby.Resample.time_agg_large_num_groups('sum', 2, False) - 117±2ms 65.3±0.7ms 0.56 groupby.ResampleCFTime.time_agg_large_num_groups('sum', 2, False) - 113±1ms 62.0±0.4ms 0.55 groupby.ResampleCFTime.time_agg_large_num_groups('sum', 1, False) - 112±2ms 61.4±0.5ms 0.55 groupby.Resample.time_agg_large_num_groups('sum', 1, False) - 8.50±0.2ms 1.61±0.01ms 0.19 combine.Combine1d.time_combine_by_coords - 1.81±0.02s 224±2ms 0.12 combine.Combine1dDask.time_combine_by_coords

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY. PERFORMANCE INCREASED. ```

If we can find cases where we know concurrent writes are unsafe, we can definitely start raising errors. Better to be safe than to suffer from silent data corruption!

let's have a look at what defines the object: python data = xr.DataArray(np.random.randn(2, 3), dims=("x", "y"), coords={"x": [10, 20]}) That means we have: - random data - two dimensions: x with a length of 2 and y with a length of 3 - one coordinate: x with values [10, 20]

The line python data.x.attrs["units"] = "x units" can be split into two parts: python x = data.x and python x.attrs["units"] = "x units" where x is the coordinate.

If we did the same thing with y we would not get the dimension, but a dummy coordinate that has the values np.arange(data.sizes["y"]). This is admittedly a bit confusing.

Thanks! I tracked this down to the difference between reading the file remotely, or downloading first and accessing a local copy on v0.20.2 (the latter is what I used to produce my figures). Can you reproduce? remote = xr.open_dataset( "http://kage.ldeo.columbia.edu:81/SOURCES/.LOCAL/.sst.mon.mean.nc/.sst/dods" ).sst.sel(lat=20, lon=280, method="nearest") local = xr.open_dataset("~/Downloads/data.cdf").sst.sel(lat=20, lon=280, method="nearest")

(remote.groupby("time.month") - remote.groupby("time.month").mean()).plot()

(local.groupby("time.month") - local.groupby("time.month").mean()).plot()

Yes, 10-30% on my machine: before after ratio [91f14c9b] [1eb74149] <v2023.04.2> <speedup-dt-accesor> - 1.17±0.04ms 1.02±0.04ms 0.87 accessors.DateTimeAccessor.time_dayofyear('standard') - 1.25±0.07ms 976±30μs 0.78 accessors.DateTimeAccessor.time_year('standard') - 3.90±0.1ms 2.68±0.05ms 0.69 accessors.DateTimeAccessor.time_year('noleap') - 4.75±0.07ms 3.25±0.04ms 0.68 accessors.DateTimeAccessor.time_dayofyear('noleap')

I defer to @shoyer, the solution with where_method seems good to me.

I agree, coordinate is the correct term. We could make that a bit less confusing by using a non-dimension coordinate in the example, but then again the point is that any coordinate can have additional metadata.

But I think the concept of dimension-coordinates (when the coordinate has the same name as the dimension) is still relevant.

If I remember correctly, while they lost their status as the only ones having an index dimension coordinates are still used for alignment.

Can you compare ds_anom at a point in both versions please? I get a plot that looks quite similar

v0.20.2:

v2022.03.0:

v2023.04.2

@maxhollmann I'm sorry, I'm still finding my way into Xarray. I've taken a closer look at #2377, especially https://github.com/pydata/xarray/issues/2377#issuecomment-415074188.

There @shoyer suggested to just use:

python data = duck_array_ops.where_method(data, ~mask, fill_value)

instead of python data[mask] = fill_value

I've checked and it works nicely with your test. That way we would get away without the flags test and the special handling will take place in duck_array_ops. Would be great if someone can double check.

combine_first uses fillna under the hood -> #3570