issue_comments

The more I think about this PR, the more I dislike this approach. The solution must either be brittle or over-engineered. I discussed this issue with a friend, and other approaches don't seem better: currying the function—arr.sel(method="nearest")(method="foo")—or adding a decorator that caches the results of a stripped down inspect.signature. I think the best approach is to just raise a ValueError if certain string arguments are not in a predefined set of choices:

if method not in ["nearest", ...]: raise ValueError(...)

@max-sixty, @dcherian, @shoyer thoughts?

Not to open a can of worms, but the root cause of this issue is that the Xarray API accepts both **kwargs and ordinary named args. This PR—and methods like either_dict_or_kwargs—seems like workarounds to accommodate this model. Is there any interest in moving away from this API long-term?

I'm almost done with classes and am happy to return to this if there is any interest. I don't want to design something brittle, though, and would love some guidance.

I've integrated the latest feedback and created https://github.com/pydata/xarray/issues/3356 for the section on name-matching rules.

@jthielen, it's not my call, but I think a downstream PR makes sense.

Thanks for the feedback @max-sixty and @dcherian. I tried to incorporate your feedback and pushed some new changes. Here's a screenshot of the page. Happy to iterate more on this if needed. I do think getting this stuff right is really useful for new users.

Other things we should add (but not required right now): alignment, broadcasting, merging, concatenating, combining.

Ah, these would be great to have. I've created https://github.com/pydata/xarray/issues/3355 so I/we remember.

This could be an addition to the either_dict_or_kwargs helper function.

Currently, either_dict_or_kwargs doesn't have access to the DataArray instance's dims or the function's keyword arguments; are you suggesting that either_dict_or_kwargs be modified to take these? I do like this approach. Otherwise, I'd have to add a second utility function check everywhere either_dict_or_kwargs is used, and it'd be easy to forget going forward.

What if I create a PR with a function in utils that takes an array and a list of keyword arguments, and warns the user if the array has any of the keyword arguments as dimensions. The warning can suggest using a dict.

Thanks for the explanation, @DocOtak. I didn't initially understand :suppress:, but I now agree that a cleaning block with :suppress: is straightforward. It also follows the current pattern in the docs—I noticed this other places once I knew what to look for.

After doing some more research, I can't find an ipythonic way of doing this—even have a question with a bounty on StackOverflow. However, here are some options. My vote is for using the Makefile:

Remove via Makefile

It's not ideal, but it ensures the files get removed immediately after they get created or upon clean. Near the top of the file, put something like

bash ... TMP_FILES = example-no-leap.nc foo.zarr/ manipulated-example-data.nc path/ ...

Then the clean and html commands are

```bash .PHONY: clean clean: rm -rf $(BUILDDIR)/ rm -rf generated/ rm -rf auto_gallery/ rm -rf $(TMP_FILES)

.PHONY: html html: $(SPHINXBUILD) -b html $(ALLSPHINXOPTS) $(BUILDDIR)/html rm -rf $(TMP_FILES) @echo @echo "Build finished. The HTML pages are in $(BUILDDIR)/html." ```

Remove via more documentation

As @DocOtak points out, some of the documentation already cleans up after itself. Personally, I dislike this as it's not clear to the reader why these extra snippets exist.

Remove via .gitignore

Easy to do, but I dislike this as it leaves a bunch of temporary junk in my working directory. I'd rather have it cleaned up somehow.

Remove by writing to and ignoring temp directory

Every ipython directive that creates a file should place that file in tmp/ directory. Then we .gitignore this. This is a bit cleaner than the other .gitignore option. This is my second choice.

Remove using tempdir utilities

I dislike this for the same reason I dislike "Remove via more documentation": it clutters the documentation with utility snippets/code that are opaque to the reader.

Happy to create a PR for any of these, but does anyone have any preferences or a better way of doing this?

I see. This is because coordinates are just DataArray objects. So

```python

arr.coords['y'] = range(3) ```

is equivalent to

```python

new_coords = xr.DataArray(data=range(3)) arr.coords['y'] = new_coords ```

And the reason this is a ValueError is that new_coords has a default dimension dim_0 that is not on arr. However, this

```python

arr.coords['y'] = 1 ```

is equivalent to...

```python

new_coords = xr.DataArray(data=1, dims=[]) arr.coords['y'] = new_coords ```

And new_coords has no dimensions that are not on arr.

@shoyer, maybe I am missing a nuance in your answer, but I think I already understand this bit about Xarray. My question was why is it is allowed to add a scalar:

```python

arr = xr.DataArray(range(3), dims=['x'], coords={'x': list('abc')}) arr.coords Coordinates: * x (x) <U1 'a' 'b' 'c' arr.coords['y'] = 1 # <----------- Why does this work? arr.coords Coordinates: * x (x) <U1 'a' 'b' 'c' y int64 1 ```

In this case, y are coordinates that do not share an existing dimension.

@shoyer, I've addressed your comments by adding more helpful warnings and testing that said warnings are raised properly. I've also moved both warnings to the top of the function as I think it's a bit easier to reason about the code branches this way.

Strange. This passes all my tests locally.

Okay, I'll finish https://github.com/pydata/xarray/pull/3128 first and then tackle this.

Happy to make these changes, but what's the protocol here? Seems like reverting the merge and letting me amend is cleaner than me creating a new PR.

@horta, what's the status on this? @max-sixty and I were discussing a glossary over in https://github.com/pydata/xarray/issues/3176. I think this is an important contribution and am happy to help if needed.

Looks like the idea of a glossary is already being discussed in https://github.com/pydata/xarray/issues/2410.

I did not want to overreach with this PR, but I agree that the documentation is verbose. It seems to follow a logical order of information without making reasonable assumptions about the contributor. I think the docs would be better suited following an inverted pyramid of information. In that light, something like this PR checklist or an overview would be at the top with quick links to more detailed information at the bottom.

For example, while it's true that you need to know about git before contributing, how many people need to learn git or forking or creating branches when first contributing? My guess is that this is the minority, and the docs could reflect this by putting such reference material near the bottom of the page.

Also added a note to "What's new".

@max-sixty, done.

Sorry for the back-and-forth. CI tests and checks are all passing, and this is ready for another review.

Now the docs don't build. When I try to build locally, I get a vague error:

bash File "/Users/gwg/miniconda3/envs/xarray-docs/lib/python3.7/site-packages/IPython/sphinxext/ipython_directive.py", line 583, in process_input raise RuntimeError('Non Expected warning in `{}` line {}'.format(filename, lineno)) RuntimeError: Non Expected warning in `/Users/gwg/projects/xarray/doc/indexing.rst` line 241

Do you see anything wrong with this?

``rest The :py:meth:~xarray.Dataset.drop` method returns a new object with the listed index labels along a dimension dropped:

.. ipython:: python

ds.drop(['IN', 'IL'], dim='space')

<-- line 241 drop is both a Dataset and DataArray method.

Use :py:meth:~xarray.Dataset.drop_dims to drop a full dimension from a Dataset. Any variables with these dimensions are also dropped: ```

Thanks for these answers! On a related point, I'd be keen to open a PR for improved documentation for whatever object a is. It seems like the documented Xarray terminology is "multidimensional coordinate", right? To me, "non-index coordinate" and "multidimensional coordinate" are both pretty vague until you're more familiar with Xarray's way of thinking.

What do you think of the terminology "alternative" or "auxiliary" dimension? a is clearly a dimension in the sense that it has coordinates or labels for all the "tick marks" along the x dimension. At the very least, I'd love to add a lot more examples of how to actually use these things.

Sorry about that. I'm not sure what happened. I've reverted branch and cherry picked my changes. The issue with drop being a duplicated name still stands.

Looking at this now, and I'm a little surprised at the verbiage. In your example, do you consider a to be a "variable"? I thought variables were individual DataArray objects "inside" Dataset objects. My colleagues and I have been referring to objects such as a as "alternative" or "auxiliary" dimensions. Basically, a different labeling of the same coordinates. You also seem to call these "multidimensional coordinates"?

But I do think I see the use case. The point is that you can take an existing dimension's coordinates and set them as the coordinates for an alternative dimension?

@max-sixty, thanks! It's useful to be able to do in advance.

After merging from master, I get this error:

flake8 ./xarray/core/dataset.py:3466:5: F811 redefinition of unused 'drop' from line 3460

I assume this is due to https://github.com/pydata/xarray/pull/3177 cc @crusaderky. Is there a way to avoid this?

Thanks for the explanation. I'll create a PR and link to this issue this evening.

Okay, a couple changes to the original PR: we now perform a shallow copy of the returned index in DataWithCoords.get_index and Indexes.__getitem__. The latter is needed because both Dataset.indexes and DataArray.indexes returns an Indexes object. Finally, I added a bunch of tests for various index accessors.

This seems pretty robust:

```python

import pandas as pd import xarray as xr index = pd.Index(list('abcd'), name='foo') series = pd.Series(range(4), index) array = xr.DataArray.from_series(series) array.get_index('foo').name = 'bar' array.get_index('foo').name 'foo' array['foo'].to_index().name = 'bar' array['foo'].to_index().name 'foo' array.to_index().name = 'bar' array.to_index().name 'foo' array.indexes['foo'].name = 'bar' array.indexes['foo'].name 'foo' ```

Since a Dataset is just a collection of DataArray objects, the behavior propagates nicely, e.g.

```python

dataset = xr.Dataset({'myvar': array}) dataset['myvar'].to_index().name = 'bar' dataset['myvar'].to_index().name 'foo' dataset.indexes['foo'].name = 'bar' dataset.indexes['foo'].name 'foo' ```

Locally, I've moved my changes from DataArray.to_series to DataWithCoords.get_index, and my tests still pass. After work, I'll write some more tests, e.g. for for DataArray.to_index, and push.

@shoyer, it seems like this issue happens anywhere the user can modify a Pandas index. For example, the Dataset bug in my comment yesterday. Do we want all views of indexes to be immutable or do we trust the user to not do stuff like this:

```python

dataset.indexes['mutable?'].name = 'yes' dataset.indexes['mutable?'].name 'yes' ```

@max-sixty, name is just a mutable property on a Pandas index:

```python

dates = pd.date_range('01-Jan-2019', '31-Jan-2019', name='mutable?') series = pd.Series(np.random.randn(dates.size), dates) series.index.name 'mutable?' series.index.name = 'yes' series.index.name 'yes' ```

But you're right that to_series is the wrong place. It looks like to_series calls to_index, which calls get_index, which is inherited by other classes such as Dataset. For example, the bug persists with Dataset despite my fix:

```python

dates = pd.date_range('01-Jan-2019', '31-Jan-2019', name='mutable?') series = pd.Series(np.random.randn(dates.size), dates) dataset = xr.Dataset({'foo': series}) dataset.indexes['mutable?'].name = 'yes' dataset.indexes['mutable?'].name 'yes' ```

Does anyone know if get_index is the best place for this? Maybe the tests should be somewhere more generic as well.

Created PR https://github.com/pydata/xarray/pull/3128 to add support for this feature.

I've created PR https://github.com/pydata/xarray/pull/3126 to fix this issue.