issue_comments

Great I'll merge before the next release if no one else has comments. Thanks @DWesl

One option is to change decode_coords from bool to Union[bool, str] and allow decode_coords to be either "all" (this PR) or "coordinates", or True ( == "coordinates", backwards compatible). I can bring this up at the next dev meeting.

Updated to implement this proposal after getting OK at the previous meeting.

@DWesl can you take a look at the most recent changes please?

there were some substantial concerns about backwards compatibility

:( yes this is currently backward incompatible (so the next release will bump major version). There's reluctance to add yet another decode_* kwarg to open_dataset, and also reluctance to issue a warning saying that the behaviour of decode_coords will change in a future version (since this would be very common).

One option is to change decode_coords from bool to Union[bool, str] and allow decode_coords to be either "all" (this PR) or "coordinates", or True ( == "coordinates", backwards compatible). I can bring this up at the next dev meeting.

I think it's OK if we skip ancillary_variables, the CF data model does not map perfectly to xarray's data model so this kind of annoyance will always be present. Also thanks @DocOtak for that pathological example :D (now I know what those mathematicians talk about)

A method or addition to getitem on the accessor returning a Dataset with the linked variables include

This has already been implemented though it could use a lot more testing: https://cf-xarray.readthedocs.io/en/latest/examples/introduction.html#Feature:-Accessing-variables-by-standard-names

@DWesl can you put delete the "bounds" attribute and send in a PR to xarray-data?

Hi @DWesl . I sincerely apologize for not responding earlier.

Reading over this thread again, I think I agree with @shoyer these should live in encoding, because we've technically "decoded" the attribute and converted these variables to coords.

My preference is for attrs but someone else should weigh in. cc @pydata/xarray

Maybe @dopplershift , as MetPy maintainer has thoughts on this matter?

It isn't just MetPy though. I'm sure there's existing code relying on adding grid_mapping and bounds to attrs in order to write CF-compliant files. So there's a (potentially big) backward compatibility issue. This becomes worse if in the future we keep interpreting more CF attributes and moving them to encoding :/.

Since I'm doing this primarily to get grid_mapping and bounds variables out of ds.data_vars.

I'm +1 on this but I wonder whether saving them in attrs and using that information when encoding coordinates would be the more pragmatic choice.

We could define encoding as containing a specified set of CF attributes that control on-disk representation such as units, scale_factor, contiguous etc. and leaving everything else in attrs. A full list of attributes that belong in encoding could be in the docs so that downstream packages can fully depend on this behaviour.

Currently I see coordinates is interpreted and moved to encoding. In the above proposal, this would be left in attrs but its value would still be interpreted if decode_coords=True.

What do you think?

I'm a little confused on why these are in encoding and not in attrs.