Refactor time operations

[jvegreg]

Adding other operators (min, max, median) to time, seasonal and year statistics.

Required by ESMValGroup/ESMValTool#212

I keep the seasonal_mean but probably we should prefer seasonal_statistics from now on. How are we going to manage deprecation of preprocessor functions? I think we are going to have more name changes when generalizing some of them

[bouweandela]

I keep the seasonal_mean but probably we should prefer seasonal_statistics from now on. How are we going to manage deprecation of preprocessor functions? I think we are going to have more name changes when generalizing some of them

Up until now we do not deprecate stuff, but just remove it and update all the existing recipes. Diagnostic developers with open pull requests will have to update themselves. For now I think we can keep this way of working. A minor version increase will be required for ESMValTool Core when we do this kind of thing, once we have the first official release out (in alpha versions this kind of thing is still acceptable), I've pinned the ESMValTool installation so it does not go to the next minor version and the old version will keep working.

[ledm]

One of the tasks in issue #24 is to move the get_iris_analysis_operation function from _area.py to a new file, _shared.py in the preprocesor folder.

This function can "Determine the iris analysis operator from a string", like this. It would be good if the time_statistics functions also behaved like this.

[jvegreg]

One of the tasks in issue #24 is to move the get_iris_analysis_operation function from _area.py to a new file, _shared.py in the preprocesor folder.

This function can "Determine the iris analysis operator from a string", like this. It would be good if the time_statistics functions also behaved like this.

I didn't know you already have that. I will do it

[bouweandela]

Do all of these time statistics functions support all input data frequencies (provided they are higher than the frequency of the statistic funtion of course)?

[jvegreg]

Do all of these time statistics functions support all input data frequencies (provided they are higher than the frequency of the statistic funtion of course)?

Yes, there is nothing that will make them fail for any strange frequency. As long as the time coordinate is correct, they will be applied correctly

[bouweandela]

Can you also make the associated pull request in the ESMValTool repository to fix any diagnostics that use the removed preprocessor functions?

[bouweandela]

Thanks @JaroCamphuijsen for helping out with code review!

Note that we need to make a new release of ESMValCore after this PR is merged and only then merge the associated PR in the ESMValTool repository.

[bouweandela]

Computing the variance results in a crash, because this squares the units, and then the function cmor_check_data fails:

  File "/home/bandela/src/esmvalcore/esmvalcore/cmor/check.py", line 667, in cmor_check_data
    checker(cube).check_data()
  File "/home/bandela/src/esmvalcore/esmvalcore/cmor/check.py", line 160, in check_data
    self._cube.convert_units(units)
  File "/home/bandela/conda/envs/esmvaltool/lib/python3.7/site-packages/iris/cube.py", line 917, in convert_units
    new_data = self.units.convert(self.data, unit)
  File "/home/bandela/conda/envs/esmvaltool/lib/python3.7/site-packages/cf_units/__init__.py", line 1904, in convert
    (self, other))
ValueError: Unable to convert from 'Unit('K2')' to 'Unit('K')'.

[jvegreg]

Computing the variance results in a crash, because this squares the units, and then the function cmor_check_data fails:

This is tricky. We want users to use the preprocessor as much as possible, but we can not add functions that affect the units...

I think I will remove the variance and open an issue to discuss if we want this kind of functions to be available in the preprocessor and, if that is the case, when and how to apply the checks.

[jvegreg]

Note that we need to make a new release of ESMValCore after this PR is merged and only then merge the associated PR in the ESMValTool repository.

Note that this pull request will make some parts of the paper outdated. Do not merge before we get the zenodo release for the paper

[mattiarighi]

Yep, that's why I didn't... 😃

[bouweandela]

@mattiarighi Zenodo archived versions with DOIs, including a new v2.0.0b1 release for the paper have now been created. Can we continue with testing/merging this?

[mattiarighi]

Will do!

[mattiarighi]

@ledm should also give his OK before I merge.

[ledm]

I've checked out this branch and the related ESMValTool PR, ESMValGroup/ESMValTool#1189.

I ran a couple of the edited recipes, I've also glanced through the code changes. So far, I can't see any reason to delay either PR. Everything looks great. Nice work @jvegasbsc!