Implement fixes requested by ESMValTool team

[agstephens]

Required steps to manually add fixes:

• check that our data exhibits the error to be fixed
• write the xarray fixing code in daops, with associated unit tests
• add the fix class to dachar.fixes...., with associated unit tests if required
• decide/agree on content of fix metadata:
  • URL to issue, or code, or both in ESMValTool repo
  • description of the fix
  • "source" property to identify that this came from ESMValTool, maybe with release version or github commit number.
• use inventory to identify all data sets that will be affected
• generate (by hand?) fix proposals:
  1. create fix proposal per datasets, OR
  2. create fix proposal template and list of datasets file
• do we need some kind of QC when the fixes are being proposed?
• publish fix proposals:
  • Start with a one-off proposal: dachar propose-fixes -p cmip6 <json_file>
  • Adapt to: dachar propose-fixes -p cmip6 --file-list=datasets_files.txt <fix_template.json>
• process fixes as usual - to accept them

Relevant to:

ESMValGroup/ESMValCore#755

ESMValGroup/ESMValCore#787

[agstephens]

@ellesmith88 , I have assigned this to the two of us.

[agstephens]

Suggested content for source:

"source": {
    "name": "esmvaltool...",
    "version": "3.4.5",
    "comment": "any useful info",
    "url": "https:.....isssue/34"
}

[ellesmith88]

Write up of what I have done so far:

• So far I have been working with the o2 fix: https://github.com/ESMValGroup/ESMValCore/blob/master/esmvalcore/cmor/_fixes/cmip5/hadgem2_cc.py#L34-L55

• Identifying the error in our data:
  I have found that in our cmip5 data the long name and standard name of the o2 variable in the hadgem2-cc model doesn’t match the ‘corrected’ names in the fix - so this shows that the error exists in our data.

• Write xarray fixing code in daops:
  I have written a generic fix that can be used to update any of the attributes of the main variable in a dataset:
  https://github.com/roocs/daops/blob/esmval_fixes/daops/data_utils/var_utils.py#L4

• Added a fix into dachar fixes:
  https://github.com/roocs/dachar/blob/esmval_fixes/dachar/fixes/var_fixes.py#L8
  The attributes to be updated and what they should be updated to can be passed as a list of strings - each string should contain the attribute to be changed and what it should be changed to separated by a comma e.g.
  ["long_name,Dissolved Oxygen Concentration", "standard_name,mole_concentration_of_dissolved_molecular_oxygen_in_sea_water"]

• As this data doesn’t exist in the c3s-cmip5 data (and so isn't in the inventory) I found the affected datasets using glob.glob("/badc/cmip5/data/cmip5/output1/MOHC/HadGEM2-CC/*/*/*/*/*/*/o2")

• I have provided 2 different ways of proposing a fix via the cli:

1. By providing a json file e.g. https://github.com/roocs/dachar/blob/esmval_fixes/tests/test_fixes/esmval_test_fixes/o2.json
   This option could be used when you have a fix that you only need to propose for one dataset, or just a few datasets.
   The cli usage for this is dachar propose-fixes -f <json_file> but more than one json file can be provided: dachar propose-fixes -f <json_file>,<json_file2>,<json_file3>
   The fix/dataset doesn't need to be the same in these files if multiple are provided.

2. By providing a template (json format) and a list of datasets that the fix should be proposed for (in a .txt file)
   For example:
   Template: https://github.com/roocs/dachar/blob/esmval_fixes/tests/test_fixes/esmval_test_fixes/o2_template.json
   Dataset list: https://github.com/roocs/dachar/blob/esmval_fixes/tests/test_fixes/esmval_test_fixes/o2_fix_ds_list.txt
   The cli usage for this is dachar propose-fixes -t <json_template> -d <dataset_list>

   The fixes are added to the fix proposal store from the cli via these functions:
   https://github.com/roocs/dachar/blob/esmval_fixes/dachar/fixes/generate_proposals.py#L38-L73

   More than one fix can be proposed in the complete json file or the json template.

   The command-line options are checked in cli.py so that incompatible options cannot be used together:
   https://github.com/roocs/dachar/blob/esmval_fixes/dachar/cli.py#L255

• The fix proposals are checked before being added to the fix proposal store. (QC)(https://github.com/roocs/dachar/blob/esmval_fixes/dachar/fixes/generate_proposals.py#L19).
  This checks that the required fields are there and that there are no unexpected fields.

• The source property provided in the fix proposal has been updated to include extra fields:

"source": {
    "name": "esmvaltool...",
    "version": "3.4.5",
    "comment": "any useful info",
    "url": "https:.....issue/34"
}

This records the origin of the fix.

For fixes proposed by dachar - the source is currently initialised by the check that identifies that a fix is needed e.g.
https://github.com/roocs/dachar/blob/esmval_fixes/dachar/analyse/checks/coord_checks.py#L16
There may be a better place for this if it needs to be more specific.

• The proposed fixes are recorded in the fix proposal store and can be processed and published as normal.

[agstephens]

@ellesmith88 Thanks for the detailed write-up. This is looking really good.

One thought about the format of the fixes: at the moment the attribute name and value are being embodied together in a string: key,value. I can think of cases where we might need to preserve the data type of the value and therefore I reckon it is safest to represent them as dictionaries of key/value pairs rather than a comma-separated string.

By way of an example, this current fix:

    {"fixes": [
        "long_name,Dissolved Oxygen Concentration",
        "standard_name,mole_concentration_of_dissolved_molecular_oxygen_in_sea_water"
        ]
    },

would change to:

    {"fixes": [
        {"long_name": "Dissolved Oxygen Concentration"},
        {"standard_name": "mole_concentration_of_dissolved_molecular_oxygen_in_sea_water"}
        ]
    },

If we needed to use floats or ints we could then represent them in their native form, e.g.: {"quality_flag": 3"}

[ellesmith88]

@agstephens Good point, I'll update that 🙂