issue_comments

Yes, I tested this locally, and here is the PR for that suggestion: #6366

That could work. Are you set up to check that? That can be either a full repository checkout or an XArray installation you can edit.

No, in this case "coordinates" in encoding will resolve to False, so that's not the problematic step.

I think the problem is that the author of line 773 assumed that pop_to would return the value of attrs['coordinates'] if encoding['coordinates'] is not set. That is wrong and the line should be fixed to look like this instead: python coords_str = pop_to(encoding, attrs, "coordinates") or attrs.get("coordinates")

Given this: https://github.com/pydata/xarray/blob/613a8fda4f07181fbc41d6ff2296fec3726fd351/xarray/conventions.py#L782-L783 I think that should be working. This: https://github.com/pydata/xarray/blob/613a8fda4f07181fbc41d6ff2296fec3726fd351/xarray/conventions.py#L770-L779 explicitly says it should, and is probably the part where things go wrong, but it should be going wrong the same way for encoding and attrs.

I think https://github.com/pydata/xarray/blob/613a8fda4f07181fbc41d6ff2296fec3726fd351/xarray/conventions.py#L758-L768 may need to be split into two conditionals, one for attrs and one for encoding. I'm not sure how to get the continue behavior while allowing the code to work for both attrs and encoding without code duplication.

@DWesl Thanks for digging into the details of the CF! I read your post in the sense that solution (1) is the one to choose.

I have one more question though before we close this:

When setting "coordinates" as a variable attribute instead of as an encoding, the value of that variable attribute is overwritten when writing to a NetCDF file, see this example: python import xarray as xr ds = xr.Dataset( {"values": ('time', [0.0, 0.1])}, coords={ 'time': ('time', [0, 1]), 'lat': ('time', [5, 4]), 'lon': ('time', [10, 12]) }) ds['values'].attrs['coordinates'] = "time lon lat" ds.to_netcdf("test.nc") shell $ ncdump -h test.nc netcdf test { dimensions: time = 2 ; variables: double values(time) ; values:_FillValue = NaN ; values:coordinates = "lon lat" ; int64 time(time) ; int64 lat(time) ; int64 lon(time) ; } Is this intended, and if so, what is the reason?

I tried to find what the CF conventions say about including dimension coordinates (I'm using the name from scitools-iris rather than "coordinate variable" as used in the CF conventions to keep myself from getting confused) in the coordinates attribute. From what I can tell, the whole document is consistent with usually excluding dimension coordinates from the coordinates attribute. Most of the Discrete Sampling Geometry examples in appendix H seem to include the dimension coordinates in the coordinates attributes, though at least one example leaves the dimension coordinates implied rather than explicit.

From what I remember, XArray is based on the netCDF data model, rather than the CF data model, so initializing variable_coordinates[var_name] = set(variable.dims) will do the wrong thing if the dataset doesn't set one or more of its dimension coordinates (example H.2 has variables with dimensions ("station", "time"), but no variable named station. Section 4.5 makes this practice explicit). You could work around this by leaving the initialization as it stands but dropping the if coordinate_name not in variable.dims condition on including coordinate_name as part of the coordinates attribute.

1. Stick to the current logic which might be non-conformal with the CF conventions in case of "Discrete Sampling Geometries". However, users can manually fix this by setting the coordinates in encoding.

Based on this, I think doing solution one from the previous post on writing a dataset will always be consistent with CF, but assuming that netCDF files XArray reads into datasets will always follow this pattern would be a problem. I suspect there are tests for reading netCDF files with dimension coordinates included in coordinates attributes already, but haven't checked.

1. Implement a logic to recognize cases where a dataset is a "Discrete Sampling Geometry" and only then list the non-auxiliary coordinates in the variable attribute. This is a bit tricky, and I don't have the time to implement this, I'm afraid.

If you want to try solution three, almost all Discrete Sampling Geometry files must have a global attribute called featureType. Since that attribute is recommended for all Discrete Sampling Geometry files, you could declare that the presence of that attribute defines a Discrete Sampling Geometry file for XArray. However, I don't see any place that says including dimension coordinates in the coordinates attribute is required, even for Discrete Sampling Geometry files, and a few places that explicitly say dimension coordinates can be omitted from the coordinates attribute, even for Discrete Sampling Geometry files.

The references from CF on whether dimension coordinates can be included in the coordinates attribute:

The fifth paragraph of CF section five says:

If the longitude, latitude, vertical or time coordinate is multi-valued, varies in only one dimension, and varies independently of other spatiotemporal coordinates, it is not permitted to store it as an auxiliary coordinate variable.

I think this is saying that if you can represent a coordinate using just one dimension, you shouldn't use two (that is, avoid using np.tile(np.arange(10), (3, 1)) as a longitude coordinate). The other interpretation is that dimension coordinates must not be included in the coordinates attribute, which seems unlikely given that three lines later it says:

Note that it is permissible, but optional, to list coordinate variables as well as auxiliary coordinate variables in the coordinates attribute.

The first paragraph of the section on Discrete sampling geometries:

Every element of every feature must be unambiguously associated with its space and time coordinates and with the feature that contains it. The coordinates attribute must be attached to every data variable to indicate the spatiotemporal coordinate variables that are needed to geo-locate the data.

I think dimension coordinates are explicit enough to count as "unambiguously associated", even without inclusion in the coordinates attribute, since they share a name with one of the dimensions of the Discrete Sampling Geometry data variables. This seems to be made explicit in the fourth paragraph:

Auxiliary coordinate variables containing the nominal and the precise positions should be listed in the relevant coordinates attributes of data variables. In orthogonal representations the nominal positions could be coordinate variables, which do not need to be listed in the coordinates attribute, rather than auxiliary coordinate variables.

Hey @dcherian,

Thanks for your consideration! I just started to create a new branch with my suggested changes, but then I noticed that setting the coordinates attribute to include non-auxiliary dimensions by default will let a lot of unit tests fail. That made me think about this again: In fact, always setting the coordinates attribute is permissible according to the conventions. However, most users will probably be confused about the clutter and we might run into some discussions about this in the future.

In my original post, the example can easily be fixed to reflect the coordinate attributes in the conventions (https://cfconventions.org/cf-conventions/cf-conventions.html#_single_trajectory) by enforcing the attribute value in the encoding: python import xarray as xr ds = xr.Dataset( {"values": ('time', [0.0, 0.1])}, coords={ 'time': ('time', [0, 1]), 'lat': ('time', [5, 4]), 'lon': ('time', [10, 12]) }) ds['values'].encoding['coordinates'] = "time lon lat" ds.to_netcdf("test.nc") shell $ ncdump -h test.nc netcdf test { dimensions: time = 2 ; variables: double values(time) ; values:_FillValue = NaN ; values:coordinates = "time lon lat" ; int64 time(time) ; int64 lat(time) ; int64 lon(time) ; }

From here, there are several ways to proceed: 1. Stick to the current logic which might be non-conformal with the CF conventions in case of "Discrete Sampling Geometries". However, users can manually fix this by setting the coordinates in encoding. 2. Enforce to always list all non-auxiliary coordinates in the variable attribute, which is conformal with the conventions, but generates a lot of clutter. 3. Implement a logic to recognize cases where a dataset is a "Discrete Sampling Geometry" and only then list the non-auxiliary coordinates in the variable attribute. This is a bit tricky, and I don't have the time to implement this, I'm afraid.

I actually suggest to choose solution (1) and close this issue as "won't fix". What do you think?

This seems OK to me :+1: a PR would be welcome.

cc @DWesl

If you are interested in a fix, I would modify the following section: https://github.com/pydata/xarray/blob/613a8fda4f07181fbc41d6ff2296fec3726fd351/xarray/conventions.py#L726-L744 The variable_coordinates should be initialized for each variable v with the names in v.dims. Currently, the dimension names are explicitly excluded. So it seems to me like this behavior has originally been implemented on purpose because the main use of the coordinates attribute according to the CF convention is to list the auxiliary coordinates. However, the conventions also say: "Note that it is permissible, but optional, to list coordinate variables as well as auxiliary coordinate variables in the coordinates attribute." (https://cfconventions.org/cf-conventions/cf-conventions.html#coordinate-system)

So, this issue is about including the non-auxiliary coordinates by default because it's permissible in general, and it is even required for so-called "Discrete Sampling Geometries".