issue_comments

If you want you can leave a comment But the mypy CI should fail on unused ignores, so we will notice :)

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

Better to use a tolerance in the assterion testing.

This seems to be a numpy issue, mypy thinks that you cannot call np.dtype like you do.

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

For now we can simply ignore this error.

Maybe you can add a test that creates a cachingFilemanager object, then deletes it, then run gc.collect() and check somehow if it works? But no idea how pytest interferes with this or how you can ensure that there are no references to the module anymore?

The cfgrib notebook in the documentation is broken. I guess it's related to this PR. See: https://docs.xarray.dev/en/stable/examples/ERA5-GRIB-example.html

Same problem with https://github.com/pydata/xarray/issues/7841

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

You are right, that warning is exactly what is causing the issues.

That's quite a weird bug. 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. Is that a bug in pythons garbage collection?

Or does the garbage collection already start when calling del and does not wait for the completion of the __del__ method?

You can always reach out to the creator of the original PR and comment in the PR.

But it looks like this particular PR was reaching a dead end and should be completely rewritten. But anyway the reviewers left helpful remarks on how to proceed.

I think this goes a bit against the rest of xarray which discourages in place operations.

But on the other hand, both deleting a variable and extracting a DataArray exist on their own, so why not combine it.

Well that's the point, "x" is a dimension as well as a coordinate. And since dimensions are names you cannot assign anything to them. If you want you can change coordinate by dimension coordinate.

I agree that there is a lot of terminology that is somewhat similar and/or overlapping.

In math you have the basis of a vector space which when plotting in a coordinate system you sometimes call coordinates or axes. Also you often use the phrase "the x-coordinate of point p is ...", So I understand why naming an axis "coordinate" might sound reasonable.

However in xarray (or numerics in general) you often deal with data that is not aligned to any axis in a given coordinate system. Consider the following example: you have a time series of points that might be scattered randomly in your coordinate system. Now you can assign to each point a x and y coordinate. In xarray you would call the dimension of the data e.g. "time". You will need two additional time series of data points for the x and y coordinates.

In xarray this would be something like that: python da = xr.DataArray( [5, 6, 7], dims="time", coords={ "x": ("time", [1, 2, 3]), "y": ("time", [9, 8, 7]), "time": [0, 1, 2] } You can see that in this example we cannot easily define a x-y coordinate system since the data points are not on a grid. But still we can assign each data point a x and y coordinate.

Now finding a naming convention that fits both, random data points and lattices that are aligned with the coordinate system is not trivial.

That's why we choose to go with the following: - dimension: name of an axis of a nd-array, this might be a "real" axis that has any real world equivalent or something as trivial as "order in which the data has been aquisited". In the example this was "time". - coordinate: auxiliary data that can be used to identify the data values. In the example this was "x" and "y". (Basically it is a short name for "coordinate variable") - dimension coordinate: a coordinate that assigns values to the dimension directly if possible. This is the case when the name of a coordinate is the same as a dimension. In the example this was "time" with the timestamps 0, 1 and 2 (probably this should be real timestamps).

Feel free to propose some changes to the documentation such that newcomers will find it easier to understand the terminology.

Yes exactly. A dimension is only a name for an axis.

This is a bit misleading. I think the term coordinate is correct, but you can be more precise and use dimension-coordinate, see: https://docs.xarray.dev/en/stable/user-guide/data-structures.html#coordinates

Maybe we should start calling them index as well? But I think the concept of dimension-coordinates (when the coordinate has the same name as the dimension) is still relevant.

@Mehak-4545 are you still working on this? You need to change the style of your docstrings and examples. See other docstrings formatting for inspiration.

I don't know how we would implement that, it's probably not a good idea to special case all external backends within xarray.

Either the package is installed and then it works or it is not installed and then we don't know which backend/package is missing.

I am not sure what exactly is not working. The BACKEND_ENTRYPOINTS is not a public API and is only used for xarrays internal backends. In the documentation that you have linked it is explained on how to register your project, which is purely in the setup.cfg/pyproject.toml, no need to manipulate xarray internal objects.

Maybe I was not clear enough, sry for that. With remote file system I meant that you mount e.g. an NFS drive.

It seems that you are using a https protocol to transfer the data.

What happens when you specify the netcdf4 engine directly? (Assuming it is a nc4 and not nc3 file).

Which backend are you using? Is it a "real" server or simply a remote filesystem?

The error message tells that the backends are installed and can be found by xarray.

What exactly did you try to open? A local file? If yes, can you provide the name of this file?

See also somewhat related discussion here https://github.com/pydata/xarray/issues/6749

Not sure what the correct way would be.

If you consider that d f(x) / d y = 0, maybe all zeros could be considered correct as well. The same result can be obtained if one considers that the array might be broadcast to the y-dim.

Also we have to check that happens in Datasets if one of the variables does not contain this dimension, do we raise an error as well?

By the way: even for such a small contribution you can add an entry into the whats-new changelog. If you wish to do so, you can open another PR and I will quickly merge it (I was too fast merging this one...).

Thanks for this nice little PR!

@ChristmasZCY does this answer your question? If yes, feel free to close this issue .

Locally it always works, it segfaults in CI, which makes it impossible to debug.

Reviving this ancient issue here. We could add a deprecation cycle and issue a warning for dimensions with size 1.

From 2.07±0.04ms to 369±10μs seems good :)

@Illviljan, @headtr1ck, I just noticed that the CI version of mypy is pinned to <0.990, but the pre-commit hook is at 1.1.1. Is that intentional / known? Running the hook also exposes quite a few typing errors.

I think this is because of https://github.com/pydata/xarray/issues/7270

The pre-commit hook of mypy should be disabled anyway because it takes too long to run (should be actually much faster since mypy >=1).

I think we could check if the newest mypy still segfaults or not...

Actually on top of examples maybe some simple text depictions could be useful? Something like this for aligning: Values: [ A A A ] + [ B B ] -> [ A B A B A ] Index:. [ 1 3 5 ] + [ 2 4 ] -> [ 1 2 3 4 5 ]

But just throwing ideas around

Edit: that's actually not what aligning does, haha. But you get the idea.

I think the last remaining item has been fixed by https://github.com/pydata/xarray/pull/7586, but not completely sure.

Then we could close this issue :)

@dsgreen2 if you apply the last review comments this is good to go.

I think this logic is done one level above in the call stack. But yes, maybe a different name for the argument would be better.

Not sure what you are proposing here. We have extensive documentation on data indexing and selection here https://docs.xarray.dev/en/stable/user-guide/indexing.html

If this is not what you mean, please update your description and title to something more meaningful.

Oh, you can add an entry in whats-new if you want!

Is working again :)

@headtr1ck Read the docs build is failing. Is it due to issue #7703. Or the fault is on my side.

Yes, that is this issue.

The failing docs CI is caused by pydata-sphinx-theme=0.13.2 being incompatible with sphinx-book-theme=1.0.0. I think we can pin pydata-sphinx-theme!=0.13.2 until they fixed that (though maybe that should be done in a new PR).

Just saw this comment. I have opened #7703 for this

I am not sure if pinning works as pydata-sphinx-theme had some private function which sphinx-book-theme was using but now it cannot. Will probably have to wait for a new release of sphinx-book-theme.

See #7703

Thanks for the fixes. I have opened an issue about the readthedocs failing #7703, it's not an issue of this PR.

How did you test this? I just tried it with 2023.03.0 and 2022.06.0 with all other packages the same and I get the same results (the one you say is new).

It seems that this problematic code is mostly used to determine the engine that is used to finally open it. Did you try specifying the correct engine directly?

I think some backends rely on this magic number to determine the exact file format. Not sure if this change will cause problems if one doesn't get the full 8 bytes?

Thanks for this PR!

As said above, you don't have to change code to make it work. The intended way of using is da.curvefit(...., kwargs={"method": "trf"}).

You can submit a PR which changes **kwargs to kwargs in the docstring (both, Dataset.curvefit and DataArray.curvefit) and probably add some more info there.

@StanczakDominik are you still motivated to work on these tests?

Actually, as @kmuehlbauer pointed out, we can address this for now by updating the docstring of the function. It should get clear the the extra kwargs are passed as a dict and not as **kwargs.

I am trying out open source for the first time through Outreachy. Could you please guide me a little so that I could proceed with this issue.

Welcome to xarray! I think this issue requires some discussion before we can actually address code changes. It would be better to start with a different issue first.

I think we should continue the discussion in https://github.com/pydata/xarray/issues/7130 since it is not clear which function signature (kwargs vs **kwargs) is preferred.

Thanks for this contribution!

In https://github.com/pydata/xarray/blob/main/doc/whats-new.rst

Add a line under the documentation section explaining what you have added. Use the rest of the file as a guideline on how to format.

Last call for reviews :)

@harshitha1201 feel free to add an entry in doc/whats-new.rst. Then we can merge this. Thanks!

About the logo, this might be relevant: https://github.com/xarray-contrib/xarray.dev/issues/66

The error of mypy is real, but unfortunately this is a design error on our side, which is out of scope for this PR. see the mypy message:

xarray/backends/netCDF4_.py:563: error: Signature of "open_dataset" incompatible with supertype "BackendEntrypoint" [override]

so simply add a # type: ignore[override] at the end of this line.

Does anyone know why those take so long only on macos (and, partially, windows)? Does that have to do with the runner hardware?

Some of there files are 10 numbers, so the netcdf should be ~50kb or so (mostly the header overhead). On no hardware this should take >1min to read and write?

I just released Cubed 0.6.0 which doesn't have a dependency on Dask, so this should be possible now.

Does this mean my comment https://github.com/pydata/xarray/pull/7019#discussion_r970713341 is valid again?

Can you see if you can run import numba in your environment?

If this also raises an error, your numba installation is probably broken.

If this works, then try import sparse.

Anyway, please add the output of xr.show_versions() to your issue.

I guess we should revert this change and use our own definition as proposed by the numpy defs. If I find time I'll make a new PR such that we can be independent off numpys version in this regard.

Found as an answer to a proposal PR: https://github.com/pydata/xarray/pull/7628#discussion_r1140211918

The solution is to use .str.format the other way around as you propose:

You can try das.str.format(da). Maybe you need to use {} instead of % in your format array.

That issue will be fixed by #7619

Again, I think that this implementation is too specific and the issue wanted a more general string formatting option.

Also, auch a method is better kept as part of the StrAccessor.

Could you also merge main again?

Could this potentially affect performance?

By the way, this works (but probably slow?):

```python

def tostr(x): return f"{:.2f}"

tostr_v = np.vectorize(tostr)

da = xr.DataArray([1, 2, 3.456], dims="x")

tostr_v(da) ```

Wouldn't it be more useful to have something like da.str.format("sometext: {:2.2f}")

Or maybe da.as_str(...)

@melioristic could you reproduce the matplotlib results by setting interpolation?

Then let's close this issue in favor of #4817

@dopplershift does this answer fix your problem?

Nice! (at one point we can consider removing infer_intervals as this is now also done in matplotlib and cartopy).

Yeah, I was wondering why we do this explicitly. Should we remove it now already, or is there something that still requires it (like older matplotlib versions etc)?

I assume this has been resolved then? If not, feel free to reopen.

Thanks for the report and sorry for the late reply :)

This is not an issue of xarray but rather netCDF4. The corresponding issue can be found here: https://github.com/Unidata/netcdf4-python/issues/941 It seems that there is no solution yet.

Since it seems that it is a HDF5 issue I don't know if any other backend could fix this issue for you (I tried h5netcdf without success).

I will close this issue for now since we cannot fix it anyway.

This is indeed intentional (Now with type hints even more obvious). I agree that in a jupyter notebook that automatically displays the last return value, this might look confusing.

Almost all files have from __future__ import annotations now and pyupgrade is doing its thing :)

The logic of figuring out what stays and what goes is different in DataArray and Dataset. DataArray reduces the variable and then kicks out all dims, coords, indexes that don't fit anymore. Dataset tries to do the selection on the fly (and apparently fails to do so).

This is actually a bug in Dataset.reduce, it will drop the time index as well. See this example: ```python da = xr.DataArray([[1, 2, 3], [4, 5, 6]], dims=["x", "y"], coords={"x": [5, 6]}) da.reduce(lambda x, _, *__: x, dim="x") # has time coord

ds = xr.Dataset({"a": da}) ds.reduce(lambda x, _, *__: x, dim="x") # lost time coord ```

Probably the assumtion when writing this code was that the passed function actually reduces the dim.

Closing for now. Feel free to reopen or better to comment in the linked issues above.

Closing for now. Seems like an upstream issue. Feel free to reopen if it is still relevant.

Am I missing something, or is there no IndexVariable.name attribute anymore?

Sounds like a good idea to mention the license and what it means in the documentation.

We don't assign issues to people, so if you are interested in doing so feel free to open a PR.

DataAlignable = TypeVar("DataAlignable", bound=DataWithCoords | Coordinates) -> doesn't work since we cannot mix DataWithCoords and Coordinates when aligning each object (input type = output type)

I think thats exactly correct. The whold idea of using a TypeVar for the inputs means that all inputs must have the same type.

Consider the following example: ```python class A: ... class B(A): ... class C: ...

T = TypeVar("T", bound=Union[A, C]) def f(*x: T) -> T: ...

f(A(), A()) # OK f(A(), B()) # OK f(C(), C()) # OK f(A(), C()) # not ok, because the common type of A and C is object ``` Also: sorry for the late reply.

Closing this issue now as completed. Discussion about documentation can be continued in #7344

Maybe it is just a problem of documenting it more clearly?

Closing this issue in favor of #7593 since the provided MVCE in this issue now works.

Any ideas on how to deprecate this? Do we raise a Deprecation warning if someone uses the open_dataset method of the internal backend? And then remove it some versions down the line?

It seems that numpy is somewhat against this change, so maybe we are better off to restore the previous state to not have a mess with supporting different numpy versions again. Lets keep that in mind before the next release but wait if something changes on numpys side.

Ok :) But I guess it is better to not merge this then, it will only create messy merge conflicts for other PRs.

Is that still wanted? It seems like a useful feature to have.

Good spot. The fix seems good to me. If you fix the tests and add a whats-new this is good to go

As a newcomer, a short general question: The 'Contributing' guide mentions to document changes in whats-new.rst. Shall this be done by the developer or are you (the maintainers) doing this once the PR is merged into the main branch?

(Since at the time of requesting, I can`t know when (or if) the issue will be merged).

Usually this is done by the developer/contributor. You can just copy a similar entry and adopt it to yourself in the newest section, you have basically full freedom :)

I have opened https://github.com/numpy/numpy/issues/23308

It should be an easy change on numpys side if they decide to go for it. Only problem, we again have to special case this based on bumpy version, haha.

Should we special case a solution? Seems user-friendly to do so.

I think dict_keys is its own type so with some type ignore this could work.

Do you know if there's a plan to make something like this public on the numpy end?

No idea, the static typing of numpy is still changing a lot and has many bugs (same with xarray, haha).

I'm sure they will make it public is we request it.

I will not be able to join tomorrow, so here are my thoughts on this topic:

• How to work with subclassed Datasets that should return DataArrays? I think the pandas approach is good, we add Dataset.dataarray_cls = DataArray and DataArray.dataset_cls = Dataset which subclasses can overwrite.
• How to deal with subclasses that change the constructor aka. __init__? Probably we need to introduce a kind of shortcut constructor that will be called instead in functions that create new instances (right now the subclass needs the same constructor signature as xarrays version). But then custom instance attributes will not be set etc. Not sure what is the best approach here.
• Is the whole thing limited to DataArrays and Datasets or can one also subclass Variables?

@liorella-qm in the meantime you can use da.plot.line(col='y') as a workaround

This is caused by the squeeze() here: https://github.com/pydata/xarray/blob/41949209b6cfb0d5c25d000b1e0c5739b513385c/xarray/plot/dataarray_plot.py#L267

@Illviljan do you know why this is required? I assume you want to remove singleton dimensions to better choose the correct plot method?

Maybe we should exclude col, row dims from the squeeze? something like: darray.squeeze(d for d, s in darray.sizes.items() if s == 1 and d not in (row, col, hue))

I am a bit puzzled here... The dask graph looks identical, so it must be the way the indexers are constructed.

The major difference I can find is: The old version used np.unique while the new version uses xarrays cond.any(..)

Maybe someone with more experience in dask can help out?

Git bisect pinpoints this to https://github.com/pydata/xarray/pull/6690 which funny enough, is my PR haha. I will look into it when I find time :)

Can confirm, on my machine it went from 520ms to 5s

Not sure if related, but this also randomly failed (different error though): https://github.com/pydata/xarray/actions/runs/4276072684/jobs/7443939252