Expand functionality

[Alessimc]

Summary:

• Updated elements in mmd_elements.yaml
• Implemented a way to include non mmd elements
• Removed CF table from template
  • external_variables is the only elements left in cf_elements.yaml that has not been included in mmd_elements.yaml
• Added mmd_to_adc.py to generate adc_elements.py

Related issue:

Suggested reviewer(s):

Reviewer checklist:

• The headers of all files contain a reference to the repository license (i.e., "License: This file is part of py-mmd-tools, licensed under the Apache License 2.0 (https://www.apache.org/licenses/LICENSE-2.0)")
• 100% test coverage of new code - meaning:
  • The overall test coverage increased or remained the same as before
  • Every function is accompanied with a test suite
  • Tests are both positive (testing that the function work as intended with valid data) and negative (testing that the function behaves as expected with invalid data, e.g., that correct exceptions are thrown)
  • Functions with optional arguments have separate tests for all options
• Examples are supported by doctests
• All tests are passing
• All names (e.g., files, classes, functions, variables) are explicit
• Documentation (as docstrings) is complete and understandable

The checklist is based on the S-ENDA conventions and definition of done (see https://s-enda-documentation.readthedocs.io/en/latest/general_conventions.html). The above points are not necessarily relevant to all contributions. In that case, please add a short explanation to help the reviewer.

[mortenwh]

Thanks - it looks promising. See inline comments. In addition, flake errors should be resolved, and the coverage should be brought back to 100%.

[mortenwh]

This is ACDD elements, right? Or are there additional ones from/for ADC? Maybe better to call it "ACDD_elements.yaml"?

[Alessimc]

According to https://adc.met.no/node/4 there are five attributes that are "Global attributes not being part of ACDD, but that are parsed" by ADC. This would be:

• iso_topic_category
• activity_type
• operational_status
• featureType
• wigos_id (as you mentioned this can be removed anyway)

But I could call it ACDD_elements.yaml and maybe add a disclaimer in the comments for these atrributes?

[mortenwh]

The task is focused on making a yaml for ACDD isn't it? I suggest taking out those elements that are not ACDD, and as you suggest, call it ACDD_elements.yaml. Not sure about the others but featureType at least is CF and is already in cf_elements.yaml. mmd_elements.yaml should cover the ADC needs as far as I know (also the wigos_id).

[Alessimc]

Originally I thought we were making a yaml for Arctic Data Centre requirements like this adc_required_global_attributes.yaml

I have now made a acdd_elements.yaml as suggested.

Previously we discussed possibly removing the cf_elements.yaml since ACDD 1.3 adopts all CF 1.6 (and presumably later versions) global attributes. All cf elements except external_variables (I couldn't find it in ACDD) has already been added to mmd_elements.yaml.

So the wigos_id needs of ADC are covered by the platform/instrument attribute in mmd_elements.yaml?

[mortenwh]

yes, the data producer should add the station url as platform_resource, e.g., https://oscar.wmo.int/surface/#/search/station/stationReportDetails/0-20000-0-01447, and use the name provided there.

similarly, the instrument should be added with name and resource from the oscar database.

[mortenwh]

if this cannot be used, we should in principle create another yaml file adc_elements.adoc containing the wigos_id field but I can't understand that this is necessary.

[Alessimc]

I have 3 tests failing:

FAILED tests/test_nc_to_mmd.py::TestNC2MMD::test_missing_platform_vocabulary - AssertionError: Lists differ: [{'Short_Name': 'Envisat', 'short_name': '[199 chars]at'}] != []
FAILED tests/test_nc_to_mmd.py::TestNC2MMD::test_platform_vocabulary_invalid_url - AssertionError: Lists differ: [{'Short_Name': 'Envisat', 'short_name': '[199 chars]at'}] != []
FAILED tests/test_nc_to_mmd.py::TestNC2MMD::test_polygon_is_not_wkt - PermissionError: [Errno 13] Permission denied: '/home/alessio/MET/repos/py-mmd-tools/tests/data/reference_nc_fail.nc'

but these are also failing on the master branch.

[mortenwh]

I have 3 tests failing:

FAILED tests/test_nc_to_mmd.py::TestNC2MMD::test_missing_platform_vocabulary - AssertionError: Lists differ: [{'Short_Name': 'Envisat', 'short_name': '[199 chars]at'}] != []
FAILED tests/test_nc_to_mmd.py::TestNC2MMD::test_platform_vocabulary_invalid_url - AssertionError: Lists differ: [{'Short_Name': 'Envisat', 'short_name': '[199 chars]at'}] != []
FAILED tests/test_nc_to_mmd.py::TestNC2MMD::test_polygon_is_not_wkt - PermissionError: [Errno 13] Permission denied: '/home/alessio/MET/repos/py-mmd-tools/tests/data/reference_nc_fail.nc'

but these are also failing on the master branch.

The master branch seems good. Maybe it's just a local problem?

[mortenwh]

I have 3 tests failing:

FAILED tests/test_nc_to_mmd.py::TestNC2MMD::test_missing_platform_vocabulary - AssertionError: Lists differ: [{'Short_Name': 'Envisat', 'short_name': '[199 chars]at'}] != []
FAILED tests/test_nc_to_mmd.py::TestNC2MMD::test_platform_vocabulary_invalid_url - AssertionError: Lists differ: [{'Short_Name': 'Envisat', 'short_name': '[199 chars]at'}] != []
FAILED tests/test_nc_to_mmd.py::TestNC2MMD::test_polygon_is_not_wkt - PermissionError: [Errno 13] Permission denied: '/home/alessio/MET/repos/py-mmd-tools/tests/data/reference_nc_fail.nc'

but these are also failing on the master branch.

The master branch seems good. Maybe it's just a local problem?

No - I get the same. Quite surprising, as this should not happen. I'll look into it

[mortenwh]

#340 - do you want to try @Alessimc ?

[mortenwh]

I have solved it: #341

We can update this branch when that is merged

[mortenwh]

@Alessimc - the tests are passing now, except coverage. Can you add tests for the new code?

[Alessimc]

@Alessimc - the tests are passing now, except coverage. Can you add tests for the new code?

Absolutely! I will try to look into this later today.

Sorry for some late responses. My work hours are reduced now that university courses have started again.

[mortenwh]

@Alessimc - the tests are passing now, except coverage. Can you add tests for the new code?

Absolutely! I will try to look into this later today.

Sorry for some late responses. My work hours are reduced now that university courses have started again.

No problem, I see some tests are still failing but you can pick up this when it suits you.