propagation of encoding

[keewis]

What is your issue?

We frequently get bug reports related to encoding that can usually be fixed by clearing it or by overriding it using the encoding parameter of the to_* methods, e.g.

• wrong time encoding after padding #4224
• Error when rechunking from Zarr store #4380
• ComplexWarning: Casting complex values to real discards the imaginary part #4655
• DOC: zarr note on encoding #5427
• Nan/ changed values in output when only reading data, saving and reading again #5490
• 'numpy.datetime64' object has no attribute 'year' writing from grib2 source fsspec/kerchunk#130

There are also a few discussions with more background:

• Zarr chunking fixes #5065 (comment)
• Rules for propagating attrs and encoding #1614
• Move encoding from xarray.Variable to duck arrays? #5082
• ENH: Add keep_encoding to global options #5336

We discussed this in the meeting yesterday and as far as I can remember agreed that the current default behavior is not ideal and decided to investigate #5336: a keep_encoding option, similar to keep_attrs, that would be True (propagate encoding) by default but will be changed to False (drop encoding on any operation) in the future.

cc @rabernat, @shoyer

[jhamman]

See also #7686. The ideas presented here are also great!

[jhamman]

This issue was discussed at this week's dev meeting. I will summarize what we discussed:

1. General agreement that propagating encoding through arbitrary operations (e.g. slice, chunk, computation) leads to inconsistent states that are hard to protect against. This often leads to problems when serializing datasets in our backends.
2. The primary benefit of keeping encoding on Xarray objects is the ability to exactly roundtrip datasets. However, this benefit is less obvious after a dataset has been modified.
3. We currently have two APIs for setting encoding (e.g. to_netcdf(..., encoding={...}) and ds.encoding = {...}). We should change this by deprecating setting encoding on Xarray objects using the .encoding property.
4. We can move towards providing utilities that expose a dataset's source encoding (e.g. open_dataset(..., return_encoding=True).

Specific action items that can happen now:

• add reset_encoding to Dataset/DatAarray api (Add reset_encoding to Dataset and DataArray objects #7686)
• add a DeprecationWarning to the @property.setter for encoding on Dataset/DatAarray/Variable
• document the change in callout in the Xarray user guide.

Longer term action items:

• add option to backend readers to keep / discard interpreted encoding attributes
• disable all encoding propagation by discarding encoding attributes once a Dataset has been modified.

[rabernat]

We should also consider a configuration option to automatically drop encoding.

[klindsay28]

In the hypothetical invocation open_dataset(..., return_encoding=True), do you envision the returned encoding as being a separate returned object, or would it still be an attribute on the Dataset object?
I'm guessing the latter, because the subsequent statement 'disable all encoding propagation by discarding encoding attributes once a Dataset has been modified' doesn't make much sense to me for the former.
If so, after encoding attributes are discarded, would there still be an encoding attribute on the Dataset object that the user could reset to the values prior to the Dataset modification? This would enable the user to propagate encoding values through their workflow.

[shoyer]

In the hypothetical invocation open_dataset(..., return_encoding=True), do you envision the returned encoding as being a separate returned object, or would it still be an attribute on the Dataset object?

My expectation was that this would be a separate object, e.g., dataset, encoding = xarray.open_dataset(..., return_encoding=True), where encoding is a dict providing the encoding on each variable, and which could be passed as the encoding argument into to_netcdf(). That said, I can see how keeping encoding as variable attributes could also be convenient.

"disable all encoding propagation by discarding encoding attributes once a Dataset has been modified" would be an intermediate step, on the route to removing encoding from Xarray's data model entirely entirely.

(As a side note, I would probably spell this as open_dataset_with_encoding rather than having a function with a variable return signature.)

[klindsay28]

In a future where encoding has been removed from Xarray's data model entirely, would open_dataset_with_encoding, or whatever name gets settled on, still exist? It's not clear to me if removal from the data model means just removing it from Xarray's data structures, or if it also means removing it from Xarray's APIs.

[Metamess]

My expectation was that this would be a separate object, e.g., dataset, encoding = xarray.open_dataset(..., return_encoding=True), where encoding is a dict providing the encoding on each variable, and which could be passed as the encoding argument into to_netcdf(). That said, I can see how keeping encoding as variable attributes could also be convenient.

For your consideration, I would like to posit the following use case:
In some part of a larger application, new datasets are created through various means. Such a Dataset might move through any number of functions within the application, being passed either as a source of data, or with the intention to be transformed or appended to in some way. Finally, this Dataset reaches a function where it is written to a file. In any of the steps, from the creation to the intermediate processing to the final write stage, some encoding properties might be determined and added to the Dataset.

From this point of view, the encoding settings of the Dataset is logically an attribute of the Dataset and its elements. It would also be a pain (and lead to a degradation in code quality) to have to add a dataset_encoding parameter to all of these functions, and to modify all their return type signatures to a tuple of Dataset, dict, just to make sure the encoding gets propagated alongside the Dataset.

"disable all encoding propagation by discarding encoding attributes once a Dataset has been modified" would be an intermediate step, on the route to removing encoding from Xarray's data model entirely entirely.

My two cents: As a user, I would not expect arbitrary functions applied to a Dataset to also remove all encoding attributes. In fact, it would probably send me on a debug journey to figure out how, why and when my Dataset suddenly lost all the encoding settings I had added to it. Arguably, the clearing of encoding would be a side-effect, and one that most operations should not have.

If I understand correctly, in the end the properties stored in the encoding attribute are meant for a backend function/library that will write the Dataset to a file (like Zarr, or NetCDF, or even some custom format through a self defined function). The actual effect of these properties come from the meaning that these backends assign to them. Therefore I would not, as Xarray, make assumptions about what functions invalidate what properties of the encoding attribute, but leave this to the user. So perhaps a reasonable approach could be to let the encoding attribute exist, but to not have any Xarray functions add, delete or modify them. If a user performs a function that impacts the encoding, they should fix those values before attempting to write to a file. (For these purposes, I would consider functions like open_zarr to be 'backend' functions, which can add properties to the encoding attribute matching those that their to_file counterpart would ingest)

As long as the documentation is clear on this behavior, I believe anyone encountering encoding related issues should be able to figure out that they have to fix the encoding attributes causing the issue.

I hope this is a helpful contribution to the discussion :)

[max-sixty]

Even before going through the items in #6323 (comment) — would it make sense to at least remove the old encoding attribute on .chunk? Or at least encoding["chunks"]?

(Possible we could have pushed #8069 towards this, rather than setting a new encoding attribute? Thanks again to @Metamess for starting that PR...)