Scatter vs. Lines:

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

Thanks @mgunyho !

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

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?

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]: {} ```

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

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. ```

I've noticed this as well, see #7824.

Thanks, @mgunyho !

Indeed is pint.

Lots of pint errors with version 0.21, @keewis. I think pint 0.20.1 worked well?

Feels like it worked quite recently, has something changed? Benchmarks still works on the flox side.

Thanks, @ksunden and @tacaswell for the guidance we'll tackle these in separate PRs, see #7802.

Thanks, @dstansby !

Those issues indeed has to be fixed if opening files lazily is the only option for xarray.

But xarray could also accept that chunks=None will (for now) load all the files to memory. If that's ok we can merge this now I believe. I suspect there are a few in-memory users out there that could make use of this.

Why/where do you get a cftime._cftime.Datetime360Day ? It is deprecated according to cftime.

It's because opening several files requires concatenating the files. xarray does not have any machinery to do that lazily without dask, so all your files will be loaded to memory. Maybe that's ok for you? If the files are small it should be fine.

See #5704 for more discussion.

Realized I actually wanted to run mypy with the upstream packages. So added such a CI as well, but only when adding this label so that it doesn't mess with the scheduled runs and those results.

I notice quite a few mypy errors from the plot parts. If you have any ideas @headtr1ck and @ksunden you're very welcome to push more PRs!

The mypy 3.9 CI keeps installing the old version which hinders installing a new version with python -m pip install mypy.

Using python -m pip install mypy --force-reinstall seems to do the trick.

Before: ``` xarray/core/utils.py:116: error: Unused "type: ignore" comment xarray/core/combine.py:374: error: Unused "type: ignore" comment xarray/core/rolling.py:379: error: Unused "type: ignore" comment xarray/core/rolling.py:756: error: Unused "type: ignore" comment xarray/tests/test_plot.py:2045: error: Argument "col" has incompatible type "str"; expected "None" [arg-type] xarray/tests/test_plot.py:2054: error: Argument "col" has incompatible type "str"; expected "None" [arg-type] xarray/tests/test_variable.py:65: error: "staticmethod" expects 2 type arguments, but 1 given [type-arg] xarray/tests/test_concat.py:543: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked] Generated Cobertura report: /home/runner/work/xarray/xarray/mypy_report/cobertura.xml Installing missing stub packages: /home/runner/micromamba-root/envs/xarray-tests/bin/python -m pip install types-PyYAML types-Pygments types-babel types-colorama types-paramiko types-psutil types-pytz types-pywin32 types-setuptools types-urllib3

Generated Cobertura report: /home/runner/work/xarray/xarray/mypy_report/cobertura.xml Found 7 errors in 5 files (checked 138 source files) ```

Thanks, @kmuehlbauer !

I'm not sure about the rest of the errors, @dcherian. Maybe IndexVariable needs to use the DataWithCoords mixin?

xarray/core/groupby.py:577: error: Value of type variable "DataAlignable" of "align" cannot be "Union[DataArray, IndexVariable]" [type-var] xarray/core/groupby.py:577: error: Value of type variable "DataAlignable" of "align" cannot be "Union[Dataset, DataArray, IndexVariable]" [type-var] xarray/tests/test_groupby.py:55: error: List item 1 has incompatible type "int"; expected "slice" [list-item]

https://github.com/pydata/xarray/blob/d4db16699f30ad1dc3e6861601247abf4ac96567/xarray/core/alignment.py#L581-L588

https://github.com/pydata/xarray/blob/d4db16699f30ad1dc3e6861601247abf4ac96567/xarray/core/alignment.py#L31

https://github.com/pydata/xarray/blob/d4db16699f30ad1dc3e6861601247abf4ac96567/xarray/core/common.py#L376-L377

Looks like you almost got this figured out! You want to create a PR for this?

How come the benchmarks is not failing? Shouldn't it be significantly slower now?

No need to use a release version now that #7667 is merged.

Might be a good idea to use the next release version though.

Ok, it doesn't use the current PR, confirmed in #7668. But it works!

Maybe it's not using this PR? I'll try a merge.

PR labeler error is unrelated, see #7667.

Am I missing something obvious again?

python xarray/core/computation.py:12:1: UP035 `typing.AbstractSet` is deprecated, use `collections.abc.Set` instead xarray/core/merge.py:5:1: UP035 `typing.AbstractSet` is deprecated, use `collections.abc.Set` instead xarray/core/parallel.py:7:1: UP035 `typing.DefaultDict` is deprecated, use `collections.defaultdict` instead

Isn't it odd that ruff doesn't automatically fix this?

Probably from #7494. encode_cf_variable only accepts Variables. Replace python data = encode_cf_variable(out_data).values.astype(numpy_dtype) with

python data = encode_cf_variable(out_data.variable).values.astype(numpy_dtype) should fix the error.

mypy should have caught this a while ago when #7374 went in, does out_data have defined type hints?

Hmm, did I mess something up? I believe I only changed typing related things.

_bins suffix seems to have disappeared: ``` def test_groupby_bins_multidim(self): array = self.make_groupby_multidim_example_array() bins = [0, 15, 20] bin_coords = pd.cut(array["lat"].values.flat, bins).categories expected = DataArray([16, 40], dims="lat_bins", coords={"lat_bins": bin_coords}) actual = array.groupby_bins("lat", bins).map(lambda x: x.sum())

  assert_identical(expected, actual)

E AssertionError: Left and right DataArray objects are not identical E Differing dimensions: E (lat_bins: 2) != (lat: 2) E Coordinates only on the left object: E * lat_bins (lat_bins) object (0, 15] (15, 20] E Coordinates only on the right object: E * lat (lat) object (0, 15] (15, 20] ```

Error: [ 89.17%] ··· groupby.ResampleDask.time_binary_op_1d failed Error: [ 89.17%] ···· Traceback (most recent call last): File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 1293, in main_run_server main_run(run_args) File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 1167, in main_run result = benchmark.do_run() File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 573, in do_run return self.run(*self._current_params) File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 669, in run samples, number = self.benchmark_timing(timer, min_repeat, max_repeat, File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 705, in benchmark_timing timing = timer.timeit(number) File "/home/runner/work/xarray/xarray/asv_bench/.asv/env/e3a5540da3d30da735a9fb168f264be6/lib/python3.10/timeit.py", line 178, in timeit timing = self.inner(it, self.timer) File "<timeit-src>", line 6, in inner File "/home/runner/work/xarray/xarray/asv_bench/benchmarks/groupby.py", line 127, in time_binary_op_1d raise NotImplementedError NotImplementedError asv: benchmark failed (exit status 1)

Comparing to https://github.com/pydata/xarray/blob/5043223ca7942c6eb582798aafa843d2efc0895b/asv_bench/benchmarks/init.py#L72-L74

It has to be initialized? Try adding a short description with why it's not implemented/link to issue.

Ahh, there are a few errors in the benchmarks. Are those fixed with your other PRs? Error: [ 88.75%] ··· ...by.ResampleDask.peakmem_groupby_binary_op_2d failed Error: [ 88.75%] ···· Traceback (most recent call last): File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 1293, in main_run_server main_run(run_args) File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 1167, in main_run result = benchmark.do_run() File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 573, in do_run return self.run(*self._current_params) File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 842, in run self.func(*param) File "/home/runner/work/xarray/xarray/asv_bench/benchmarks/groupby.py", line 136, in peakmem_groupby_binary_op_2d self.ds2d.resample(time="48H") - self.ds2d_mean File "/home/runner/work/xarray/xarray/asv_bench/.asv/env/e3a5540da3d30da735a9fb168f264be6/lib/python3.10/site-packages/xarray/core/_typed_ops.py", line 589, in __sub__ return self._binary_op(other, operator.sub) File "/home/runner/work/xarray/xarray/asv_bench/.asv/env/e3a5540da3d30da735a9fb168f264be6/lib/python3.10/site-packages/xarray/core/groupby.py", line 601, in _binary_op raise ValueError( ValueError: incompatible dimensions for a grouped binary operation: the group variable '__resample_dim__' is not a dimension on the other argument asv: benchmark failed (exit status 1)

I think we should stick to default black values. Otherwise I want to start arguing for line width=79 so we follow pep8. ;)

You can still run this cleanup though so you still get your LOCs. :)

I don't enjoy using git so I'll plug Github Desktop. I think it will reduce so much of the friction git causes to beginners: https://www.youtube.com/watch?v=l7uo1d3R0Wo https://www.youtube.com/watch?v=qUYkRWGWntE

Seems we're experiencing this issue now: https://github.com/pydata/pydata-sphinx-theme/issues/1220

This one is failing on CI / ubuntu-latest py3.9 bare-minimum, is it taking a path without flox installed perhaps?

``` __ TestDataArrayGroupBy.testgroupby_fastpath_for_monotonic ___ [gw2] linux -- Python 3.9.16 /home/runner/micromamba-root/envs/xarray-tests/bin/python

self = <xarray.tests.test_groupby.TestDataArrayGroupBy object at 0x7fbd9f461c10>

def test_groupby_fastpath_for_monotonic(self):
    # Fixes https://github.com/pydata/xarray/issues/6220
    index = [1, 2, 3, 4, 7, 9, 10]
    array = DataArray(np.arange(len(index)), [("idx", index)])
    array_rev = array.copy().assign_coords({"idx": index[::-1]})
    fwd = array.groupby("idx", squeeze=False)
    rev = array_rev.groupby("idx", squeeze=False)

    for gb in [fwd, rev]:
        assert all([isinstance(elem, slice) for elem in gb._group_indices])

    assert_identical(fwd.sum(), array)

  assert_identical(rev.sum(), array_rev.sortby("idx"))

E AssertionError: Left and right DataArray objects are not identical E
E Differing values: E L E array([0, 1, 2, 3, 4, 5, 6]) E R E array([6, 5, 4, 3, 2, 1, 0]) E Differing coordinates: E L * idx (idx) int64 10 9 7 4 3 2 1 E R * idx (idx) int64 1 2 3 4 7 9 10 ```

Nice, I was looking at the real example too, Temp_url = 'http://apdrc.soest.hawaii.edu:80/dods/public_data/WOA/WOA13/5_deg/annual/temp' etc.., and it was triggering a load in set_dims:

I don't have a better idea than to do DuckArray = Any # ndarray/cupy/sparse etc. and add that as output, but that wont change anything mypy-wise besides making it easier for us to read the code.

• Absolute speed of xhistogram appears to be 3-4x higher, and that's using numpy_groupies in flox. Possibly flox could be faster if using numba but not sure yet.

Could you show the example that's this slow, @TomNicholas ? So I can play around with it too.

One thing I noticed in your notebook is that you haven't used chunks={} on the open_dataset. Which seems to trigger data loading on strange places in xarray (places that calls self.data), but I'm not sure this is your actual problem.

dask-core 2023.2.0 pyhd8ed1ab_0 conda-forge distarray 2.12.2 pyh050c7b8_4 conda-forge distlib 0.3.6 pyhd8ed1ab_0 conda-forge distributed 2021.4.1 py39hf3d152e_1 conda-forge Yeah looks like an old version. py.typed was around september so it makes sense that part.

I don't get the error. Distributed has a py.typed file https://github.com/dask/distributed/blob/main/distributed/py.typed And if it was py.typed issue we should have seen it in other PRs and I haven't seen one yet that has this error..

@TomNicholas added py.typed to package data, https://github.com/xarray-contrib/datatree/commit/927749a7702761491461f9bdfa8a0c1fbc244d85 But still, if distributed also needs to do this we should see this error in more PRs.

``` ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 49.2/49.2 kB 3.6 MB/s eta 0:00:00 Collecting types-docutils Downloading types_docutils-0.19.1.3-py3-none-any.whl (16 kB) Installing collected packages: types-PyYAML, types-pytz, types-docutils, types-setuptools Successfully installed types-PyYAML-6.0.12.5 types-docutils-0.19.1.3 types-pytz-2022.7.1.0 types-setuptools-67.2.0.1 xarray/tests/test_distributed.py:14: error: Skipping analyzing "distributed": module is installed, but missing library stubs or py.typed marker [import] xarray/tests/test_distributed.py:21: error: Skipping analyzing "distributed.client": module is installed, but missing library stubs or py.typed marker [import] xarray/tests/test_distributed.py:21: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports xarray/tests/test_distributed.py:22: error: Skipping analyzing "distributed.utils_test": module is installed, but missing library stubs or py.typed marker [import] Generated Cobertura report: /home/runner/work/xarray/xarray/mypy_report/cobertura.xml Installing missing stub packages: /home/runner/micromamba-root/envs/xarray-tests/bin/python -m pip install types-PyYAML types-pytz types-setuptools

Generated Cobertura report: /home/runner/work/xarray/xarray/mypy_report/cobertura.xml Found 3 errors in 1 file (checked 140 source files) ```

It is recommended to use it for lazy backends though: https://docs.xarray.dev/en/stable/internals/how-to-add-new-backend.html#how-to-support-lazy-loading

Hmm, I don't understand. Adding py.typed should be all that's needed, did that in flox and it worked great there: https://github.com/xarray-contrib/flox/pull/92

@agoodm, what you think of this version? Using xr.Variable directly seems a little easier to work with than trying to guess which type of array (cupy, dask, pint, backendarray, etc) is in the variable.

It would be nice to have a good copy/paste example for open_dataset. For example I think chunks have different defaults compared to open_zarr.

You can do var._data instead of var.data. There's been a few cases recently where self.data doesn't play so nice when reading from a file.

Feel free to start working on that PR. 👍 It looks like _contains_cftime_datetimes tries to do a similar thing as your solution so I think the change should be there.

pre-commit.ci autofix

I like these kinds of improvements :)

With ravel_chunks: before after ratio [3ee7b5a6] [e549724e] - 983M 183M 0.19 pandas.ToDataFrameDask.peakmem_to_dataframe - 2.76±0s 7.76±0.08ms 0.00 pandas.ToDataFrameDask.time_to_dataframe

With reshape before after ratio [3ee7b5a6] [02a4e97f] - 983M 183M 0.19 pandas.ToDataFrameDask.peakmem_to_dataframe - 2.78±0s 9.20±0.1ms 0.00 pandas.ToDataFrameDask.time_to_dataframe

Seems to work. :) I'll do a quick merge and continue in #7472.

[ 68.06%] ··· pandas.ToDataFrame.peakmem_to_dataframe 2.89G [ 68.19%] ··· pandas.ToDataFrame.time_to_dataframe 1.39±0.02s [ 68.33%] ··· pandas.ToDataFrameDask.peakmem_to_dataframe 983M [ 68.47%] ··· pandas.ToDataFrameDask.time_to_dataframe 2.77±0s

I'm not sure at all about this but maybe you're supposed to go back to something like this? https://github.com/pydata/xarray/pull/6834/commits/7fcc11ffb20583caf1976191997bd2a7525ac218#diff-791d93adb64d0986ac499ce1ba831cc95b4ffbde0dfe98b28d929935b05d7134L49

python from numpy.typing._dtype_like import _DTypeLikeNested, _ShapeLike, _SupportsDType From #6834.

Creating a separate environment file seems like a good idea. Feel free to push the changes you want, @keewis. I was thinking of coming back to this once 3.8 was dropped.

This has stalled for long enough. I'll merge this at the end of next week unless someone disagrees, this pretty much reverts to original ds.plot.scatter behaviour because defining less than x and y in ds.plot.scatter was never allowed before anyway.

This has stalled for long enough. I'll merge this at the end of next week unless someone disagrees, at least it is an improvement to the bugged main.

87d689a shows that we have issues with 3.8 when either of numba/cdms2/numbagg are not available, then we take a different code path that fails somehow. Hopefully with python 3.9 it's fixed.

Failing tests with only python 3.8 with windows, no numba/cdms2/numbagg:

Deprecation warning in pydap failing the docstring tests: ImportError while loading conftest '/home/runner/work/xarray/xarray/xarray/tests/conftest.py'. xarray/tests/__init__.py:64: in <module> has_pydap, requires_pydap = _importorskip("pydap.client") xarray/tests/__init__.py:51: in _importorskip mod = importlib.import_module(modname) ../../../micromamba-root/envs/xarray-tests/lib/python3.11/site-packages/pydap/client.py:52: in <module> from .net import GET, raise_for_status ../../../micromamba-root/envs/xarray-tests/lib/python3.11/site-packages/pydap/net.py:1: in <module> from webob.request import Request ../../../micromamba-root/envs/xarray-tests/lib/python3.11/site-packages/webob/__init__.py:1: in <module> from webob.datetime_utils import ( # noqa: F401 ../../../micromamba-root/envs/xarray-tests/lib/python3.11/site-packages/webob/datetime_utils.py:18: in <module> from webob.compat import ( ../../../micromamba-root/envs/xarray-tests/lib/python3.11/site-packages/webob/compat.py:5: in <module> from cgi import parse_header ../../../micromamba-root/envs/xarray-tests/lib/python3.11/cgi.py:57: in <module> warnings._deprecated(__name__, remove=(3,13)) ../../../micromamba-root/envs/xarray-tests/lib/python3.11/warnings.py:514: in _deprecated warn(msg, DeprecationWarning, stacklevel=3) E DeprecationWarning: 'cgi' is deprecated and slated for removal in Python 3.13

@jhamman, I believe you should simply remove GenericAlias and the __class_getitem__ it was a hack copied from a newer version of collections, see https://github.com/pydata/xarray/pull/7285#discussion_r1027253337.

Commit testing the hack: https://github.com/pydata/xarray/pull/7285/commits/d8bef27e54aa9e81873d5d64fca6a1d4d324ca62

I feel like this has been mentioned before.

Yes, now I recall, it was here #6894.

I think it could be interesting to try out: https://github.com/pmeier/array-protocol

Another good read: https://github.com/data-apis/array-api/issues/229

I've been thinking that a good and safe start on this issue is to replace all these raw Any's with T_DuckArray where T_DuckArray = Any. It won't help in the typing but it gives some traceability and makes it easier to determine what the intention was.

The array api standard doesn't define any nan*-functions. xarray pretty much always defaults to using nan*-function. :(

So any time an array has a __array_namespace__ we have to use our own nan-solution, like for .sum: https://github.com/pydata/xarray/blob/d6d24507793af9bcaed79d7f8d3ac910e176f1ce/xarray/core/duck_array_ops.py#L288-L295

Any thoughts? Are there any clever ways to handle the aggregations with NaNs in a generic way?

edit: numpy implementations: https://github.com/numpy/numpy/blob/main/numpy/lib/nanfunctions.py

I've found this part of the documentation a bit scary in the past because there is so much stuff to do for a simple "typo fix" so I welcome changes here!

Nowadays, since the CI runs pytest, ASV performance benchmarks, pre-commit and builds the documentation on every PR it isn't really necessary for new contributors to get it to run locally. The only thing left is setting up a new branch with git, change the code and create a PR. If the CI fails check the details of that test, correct the code, push a new commit. Setting up git can also be scary for new users but then there is github desktop which hides away all of that in a quite user friendly GUI.

Sure, if you're doing bigger changes running the specific tests locally is probably faster but at that point you're probably a more experienced contributor anyway.

Benchmark are improved if I understand the logs correctly. Unfortunately not significant enough to make ASV report it though. The ratio has to be >1.5 and the improvements on .time_open_dataset are around 1.3-1.4.

```

PR:

[ 50.85%] ··· dataset_io.IOReadCustomEngine.time_open_dataset ok [ 50.85%] ··· ======== ========= chunks
-------- --------- None 130±1ms {} 689±6ms ======== =========

[ 54.69%] ··· dataset_io.IOReadSingleFile.time_read_dataset ok [ 54.69%] ··· ========= ============= ============= -- chunks
--------- --------------------------- engine None {}
========= ============= ============= scipy 5.48±0.04ms 6.91±0.01ms netcdf4 2.93±0.04ms 4.32±0.02ms ========= ============= =============

Baseline:

[ 75.85%] ··· dataset_io.IOReadCustomEngine.time_open_dataset ok [ 75.85%] ··· ======== =========== chunks
-------- ----------- None 177±0.5ms {} 737±3ms
======== ===========

[ 79.69%] ··· dataset_io.IOReadSingleFile.time_read_dataset ok [ 79.69%] ··· ========= ============ ============ -- chunks
--------- ------------------------- engine None {}
========= ============ ============ scipy 4.47±0.6ms 5.74±0.7ms netcdf4 4.39±0.7ms 5.82±0.6ms ========= ============ ============ ```

```

PR:

[ 50.85%] ··· dataset_io.IOReadCustomEngine.time_open_dataset ok [ 50.85%] ··· ======== ========== chunks
-------- ---------- None 149±4ms
{} 797±20ms ======== ==========

[ 54.69%] ··· dataset_io.IOReadSingleFile.time_read_dataset ok [ 54.69%] ··· ========= ============ ============= -- chunks
--------- -------------------------- engine None {}
========= ============ ============= scipy 6.57±0.2ms 7.77±0.01ms netcdf4 3.71±0.1ms 6.17±0.5ms ========= ============ =============

Baseline:

[ 75.85%] ··· dataset_io.IOReadCustomEngine.time_open_dataset ok [ 75.85%] ··· ======== ========== chunks
-------- ---------- None 204±2ms
{} 857±20ms ======== ==========

[ 79.69%] ··· dataset_io.IOReadSingleFile.time_read_dataset ok [ 79.69%] ··· ========= ============ ============ -- chunks
--------- ------------------------- engine None {}
========= ============ ============ scipy 5.53±1ms 7.12±0.8ms netcdf4 4.96±0.6ms 6.74±0.8ms ========= ============ ============

```

```

PR:

[ 50.85%] ··· dataset_io.IOReadCustomEngine.time_open_dataset ok [ 50.85%] ··· ======== ============ chunks
-------- ------------ None 204±8ms
{} 1.20±0.04s ======== ============

[ 54.69%] ··· dataset_io.IOReadSingleFile.time_read_dataset ok [ 54.69%] ··· ========= ============ ============ -- chunks
--------- ------------------------- engine None {}
========= ============ ============ netcdf4 6.86±0.7ms 9.81±0.6ms scipy 6.74±1ms 9.10±1ms
========= ============ ============

Baseline:

[ 75.85%] ··· dataset_io.IOReadCustomEngine.time_open_dataset ok [ 75.85%] ··· ======== ============ chunks
-------- ------------ None 282±5ms
{} 1.20±0.04s ======== ============

[ 79.69%] ··· dataset_io.IOReadSingleFile.time_read_dataset ok [ 79.69%] ··· ========= ============ ============ -- chunks
--------- ------------------------- engine None {}
========= ============ ============ netcdf4 6.91±1ms 9.77±0.7ms scipy 6.91±0.7ms 9.11±1ms
========= ============ ============ ```

Seems to work, tested a little here #7426.

Seems bot uses labeler from main? I'll merge and see what want happens..

Timings for the new ASV-tests: ```

[ 50.85%] ··· dataset_io.IOReadCustomEngine.time_open_dataset ok [ 50.85%] ··· ======== ============ chunks
-------- ------------ None 265±4ms
{} 1.17±0.02s ======== ============ [ 54.69%] ··· dataset_io.IOReadSingleFile.time_read_dataset ok [ 54.69%] ··· ========= ============= ============= -- chunks
--------- --------------------------- engine None {}
========= ============= ============= scipy 4.81±0.1ms 6.65±0.01ms netcdf4 8.41±0.08ms 10.9±0.2ms ========= ============= ============= ```

Try updating to latest xarray and dask. dask has had some nice updates lately, https://medium.com/pangeo/dask-distributed-and-pangeo-better-performance-for-everyone-thanks-to-science-software-63f85310a36b

Thanks, @benbovy !

Hmm, this worked at some point. Must've gotten lost somewhere in one of the bigger plot refactors.

Positional arguments have been deprecated though, so you should start getting used to using keyword arguments: https://docs.xarray.dev/en/stable/whats-new.html#v2022-11-0-nov-4-2022

Add a py.typed file in datatree to fix the mypy error: https://github.com/pydata/xarray/blob/main/xarray/py.typed

Benchmark is a numba issue, probably #7306.

Mypy is real, cannot getitem a object. Try out using isinstance instead of the try/except to narrow the typing.

No benchmark is catching this? Maybe we can add a small one in https://github.com/pydata/xarray/blob/main/asv_bench/benchmarks/indexing.py ?

The mypy errors should be fixed soon I think: https://github.com/dask/distributed/issues/7378

I'm not really opposed to this change, shape and dtype uses self._data aswell.

Without using chunks={} in open_dataset? I just find it a little odd that it's not a duck_array, what type is self._data?

This test just looked so similar to the tests in #6797. I think you can do a similar lazy test taking inspiration from: https://github.com/pydata/xarray/blob/ed60c6ccd3d6725cd91190b8796af4355f3085c2/xarray/tests/test_formatting.py#L715-L727

Is that test targetting your issue with RAM crashing the laptop? Shouldn't there be some check if the values were loaded?

How did you import your data? self.data looks like this: https://github.com/pydata/xarray/blob/ed60c6ccd3d6725cd91190b8796af4355f3085c2/xarray/core/variable.py#L420-L435

I was expecting your data to be a duck_array?

``` error libmamba Could not solve for environment specs Encountered problems while solving: - nothing provides hdf5 1.8.15* needed by netcdf4-1.2.4-np110py27_1

  The environment can't be solved, aborting the operation

```

It's a bit annoying the ci isn't showing which package has netcdf4 as a dependency. Is that possible?

How about converting the dataset to dask dataframe? python ddf = ds.to_dask_dataframe() ddf.to_json(filename)

I am merging this on friday next week if no one minds. :)

Thank you @headtr1ck ! :)

Thanks, @jhamman !

Yeah it must be unrelated, I can't see how the doctest could be related to this.

Seaborn did something similar as the original but had a little check if the marker was filled or not: https://github.com/mwaskom/seaborn/blob/bf4695466d742f301f361b8d0c8168c5c4bdf889/seaborn/relational.py#L563

Thanks, @lukeconibear !

A good ol' copy/paste job works as expected though. :) I think we can discuss more elegant solutions in a follow up PR.

So now mypy is not crashing anymore? Thats weird, we should open an issue on mypy about this...

This one throws errors still: python -m mypy --install-types --non-interactive --python-version 3.8 --follow-imports=silent Did it ever crash if we used the normal mypy CI but with changed python version?

Dask is on the ignore list: https://github.com/pydata/xarray/blob/d6671dd414370d006254ba3156cb96256ce0e9c7/pyproject.toml#L31-L43

This seems to ignore the list and follow-imports doesn't seem to work either: - name: Run mypy with python3.8 # silent all imports, since external repos might not support this run: | python -m mypy --install-types --non-interactive --python-version 3.8 --follow-imports=silent

A good ol' copy/paste job works as expected though. :) I think we can discuss more elegant solutions in a follow up PR.

Yeah, sounds reasonable. Maybe the reason for the ignore disappeared once DuckArrayModule was implemented?

Mypy errors with python 3.8. Doesn't appear related to this PR at least.

``` Run python -m mypy --install-types --non-interactive --cobertura-xml-report mypy_report python -m mypy --install-types --non-interactive --cobertura-xml-report mypy_report shell: /usr/bin/bash -l {0} env: CONDA_ENV_FILE: ci/requirements/environment.yml PYTHON_VERSION: 3.8 TODAY: 2022-11-20 MAMBA_ROOT_PREFIX: /home/runner/micromamba-root MAMBA_EXE: /home/runner/micromamba-bin/micromamba Collecting types-PyYAML Downloading types_PyYAML-6.0.12.2-py3-none-any.whl (14 kB) Collecting types-paramiko Downloading types_paramiko-2.12.0.1-py3-none-any.whl (32 kB) Collecting types-pytz Downloading types_pytz-2022.6.0.1-py3-none-any.whl (4.7 kB) Collecting types-setuptools Downloading types_setuptools-65.6.0.0-py3-none-any.whl (48 kB) ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 48.9/48.9 kB 19.4 MB/s eta 0:00:00 Collecting types-cryptography Downloading types_cryptography-3.3.23.2-py3-none-any.whl (30 kB) Installing collected packages: types-setuptools, types-PyYAML, types-pytz, types-cryptography, types-paramiko Successfully installed types-PyYAML-6.0.12.2 types-cryptography-3.3.23.2 types-paramiko-2.12.0.1 types-pytz-2022.6.0.1 types-setuptools-65.6.0.0 xarray/core/types.py:73: error: "tuple" is not subscriptable [misc] Generated Cobertura report: /home/runner/work/xarray/xarray/mypy_report/cobertura.xml Installing missing stub packages: /home/runner/micromamba-root/envs/xarray-tests/bin/python -m pip install types-PyYAML types-paramiko types-pytz types-setuptools

xarray/core/types.py:75: error: "tuple" is not subscriptable [misc] xarray/core/types.py:77: error: "tuple" is not subscriptable [misc] xarray/core/types.py:79: error: "list" is not subscriptable [misc] xarray/core/merge.py:43: error: "tuple" is not subscriptable [misc] xarray/core/merge.py:44: error: "tuple" is not subscriptable [misc] xarray/core/merge.py:45: error: "tuple" is not subscriptable [misc] xarray/backends/api.py:65: error: "dict" is not subscriptable [misc]

Generated Cobertura report: /home/runner/work/xarray/xarray/mypy_report/cobertura.xml Found 8 errors in 3 files (checked 140 source files) ```

@dcherian, as you noted in that PR as well it's not just matplotlib but xarray too that does a lot of guessing already, x,y in plot2d for example. I was more concerned about consistency and believed we got closer to what the majority of the plotting functions does by guessing.

These are the coords that we currently guess in plot1d: https://github.com/pydata/xarray/blob/d6671dd414370d006254ba3156cb96256ce0e9c7/xarray/plot/dataarray_plot.py#L154 Should we remove them, (empty tuple) and let the user explicitly define all of them? If so should we remove any guessing in the other plot functions as well?

I think the CI wasn't catching this because of: https://github.com/pydata/xarray/blob/63a69fe4b2fa7a4de8a1b65826f6af4869818166/pyproject.toml#L73-L77

Markersize before PR:

Markersize after PR:

• default markersize values of widths are 18 to 72.
• plt.scatter default markersize is 36.
• plt.plot default linewidth is 6.

With the current version of the PR we would get 18 for 1-sized arrays, but 36 if markersize was undefined. This seems a bit inconsistent to me. I think we should default to 36 instead for 1-sized arrays, but this would require a bit more tweaks than the scope of the current PR.

I think we'll merge this to fix the zero division -> np.nan -> empty plots. And then I'll come up with a another PR where the size is the same.

@jorisvandenbossche, feel free to try out this PR.

Thanks, @DanielGoman !

A lot of pep8 is quite gentle with the phrasing I think: Avoid trailing whitespace anywhere. is only a recommendation. Limit all lines to a maximum of 79 characters. is also only a recommendation, we ignore this one for automation consistency.

Another nice perk with absolute paths is that you can immediately run a utils.py file and test out the functions, much faster workflow than having to do the proper import route.

Thanks @hmaarrfk !

``` before after ratio [c000690c] [24753f1f] - 3.17±0.02ms 1.94±0.01ms 0.61 merge.DatasetAddVariable.time_variable_insertion(100) - 81.5±2ms 17.0±0.2ms 0.21 merge.DatasetAddVariable.time_variable_insertion(1000)

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY. PERFORMANCE INCREASED. ``` Nice improvements. :)

I haven't fully understood why we had that code though?