calling compute_ann_mean(ds) can lead to ds.time becoming encoded about esmlab

[andersy005]

Changing aspects of ds in compute_ann_mean(ds) is unexpected behavior (to me).

We probably shouldn't alter the dataset time index without letting the user know.

For a temporary fix after computing compute_ann_mean, you can try the following:

import esmlab

ds = esmlab.utils.time.uncompute_time_var(ds, 'time')

I will patch this issue some time today.

[matt-long]

@klindsay28 thanks for raising this issue. I think we need to decide what the desired behavior is. At present, esmlab is geared toward operating on datasets where time has not been decoded, but returning datasets with time decoded.

Time must be decoded internally to enable application of the groupby methods that are the core of the data-manipulations.

I think I agree that a more intuitive behavior would be to restore the time axis to its un-decoded state by default.

[klindsay28]

Can the internal decoding of the time variable be done on a copy of the function arguments, instead of on the function arguments themselves? This would avoid the need to restore the time axis, because it was never changed it in the first place.

Following up on the suggestion of @andersy005, I took a look at the related function esmlab.utils.time.compute_time_var. I see that in addition to returning an xr.Dataset with an encoded time coordinate, it also encodes the time coordinate in dset, its xr.Dataset argument. I would not have guessed this from the function name. Maybe I would have if the function were named encode_time_var.

[matt-long]

@klindsay28, first, regarding the name of the function, I see your point, though we are doing more than encoding time: the function does actually compute time as the mid-point of the time_bounds.

We are using the xarray.core.groupby functionality for much of the computation. Your suggestion of not modifying the original dataset is a good one, but as far as I know this won't work with groupby. We would need to compute the groupby objects using the modified time axis and then subsequently apply them, but I don't think that's supported explicitly.

One option would be to keep the original time_coord_var on the dataset:

1. rather than replacing "time", we would use xarray's set_coords and reset_coords methods to make our "new" time object the time coordinate (and the old time coordinate a variable defined on that coordinate);
2. then apply the groupby using this coordinate;
3. then set/reset to restore the old time coordinate.

I have mixed feelings about this: it's nice to have the dataset returned with all the functionality of time-handling enabled (i.e., with time decoded). However, I recognize that the current behavior is somewhat mysterious and counterintuitive. I think carrying the old time coord along for the computation is a good idea regardless, so maybe we can have a flag (or better yet a config option) for how to return time.

[klindsay28]

It looks like the calls to groupby in compute_ann_mean use the argument group=time.year. The docs for groupby at xarray.Dataset.groupby
state that group 'must be the name of a variable contained in this dataset'. There are no stated restrictions that it needs to be a coordinate.

I wonder if compute_time_var could add to dset a new variable computed_time_mid that is encoded, and this could be used in the groupby calls.

[matt-long]

good point!

[andersy005]

I wonder if compute_time_var could add to dset a new variable computed_time_mid that is encoded, and this could be used in the groupby calls.

This sounds like a great approach and could come in handy for other purposes too.