Trt 556 final schema clean up #38

owenlittlejohns · 2024-09-10T20:41:59Z

Description

This PR performs the final clean-up relating to the recommendations for cleaning up the earthdata-varinfo configuration file schema (recommendations 4 to 7 here). Specifically:

Grid_Mapping_Data is removed (as this information wasn't directly used, and can be included as metadata attributes in CFOverrides.
ProductEpochs is removed (as this information wasn't directly used, and can be included as a units metadata attribute in the more CF Convention compliant manner via CFOverrides).
The schema now uses PascalCase for all attributes of the schema.

This is mostly small stuff updates, and I think it closes out the last of the proposed schema changes.

Jira Issue ID

TRT-556

Local Test Steps

Pull this branch.
Pip install all the local dependencies.
Try and parse a local NetCDF-4 or DMR file. It should all work.

# Install dependencies:
$ pip install -r requirements.txt

# Install this version of the package (optional, if running from the root of the directory):
$ pip install -e .

# Parse a file (with/without a configuration file, this is now in a Python repl):
> from varinfo import VarInfoFromNetCDF4
>
> vi_instance = VarInfoFromNetCDF4('path/to/file.nc4', short_name='<collection short name>', config_file='/path/to/config.json')

PR Acceptance Checklist

Jira ticket acceptance criteria met.
CHANGELOG.md updated to include high level summary of PR changes.
~~VERSION updated if publishing a release.~~ (This is a feature branch)
Tests added/updated and passing.
Documentation updated (if needed).

owenlittlejohns · 2024-09-10T20:44:08Z

config/CHANGELOG.md

+
+* The casing of all attributes in the configuration file schema has been
+  updated to be `PascalCase` throughout. Affected schema attributes include:
+  * `CFOverrides`


Something I've been toying with through the last couple of PRs... is CFOverrides the best name now? Part of me thinks MetadataOverrides would be better, but that feels like a big change. I'd love to hear some thoughts here.

It's only in the codebase 19 times. It is a big change, but is it better? Probably.

I think the main client impact is really the change in the configuration file section name.

I think the only other two user-facing places that would be impacted are the CFConfig.cf_overrides attribute and the CFConfig.get_cf_overrides method (I think everything else is just plumbing). If I'm honest, my guess is that those things aren't being called by too many end users.

Given that this overall effort to update the schema is going to be the most disruptive change to the package, now is the time to do it. I think I'm leaning to making this change. (And if it seems terrible, I can always revert the commit before merging this PR)

owenlittlejohns · 2024-09-10T20:46:33Z

tests/unit/test_var_info.py

        cls.namespace = 'namespace_string'
-        cls.sample_config = 'config/0.0.1/sample_config_0.0.1.json'


It felt off that some of the tests were coupled to the sample configuration file from the config directory. I've added the minimal amount of information to the test_config.json file that is used everywhere else to make all the tests pass (primarily adding a couple of short name paths, and a couple of entries in the mapping between mission and short name). Now all the tests that use a configuration file use the one in the test directory, which feels better.

owenlittlejohns · 2024-09-10T20:49:56Z

config/1.0.0/earthdata_varinfo_configuration_schema.json

-        },
-        "required": ["Grid_Mapping_Dataset_Name", "grid_mapping_name"]
-      }
-    },


Looks like a big change to the schema. The biggest contribution to the lines deleted are the attributes allowed in the Grid_Mapping_Data section, which are a limited set of attributes, whereas the CFOverrides (or MetadataOverrides?) cover a much broader array of metadata attributes, so it becomes untenable to define all the options for attributes there.

Are you losing any of that information? or was most of it copied from somewhere else in the first place? I'm ok with it all going since it can be added as a MetadataOverrides object.

All the specific attributes definitions were just copied from the CF Conventions page itself.

owenlittlejohns · 2024-09-10T21:10:13Z

config/1.0.0/earthdata_varinfo_configuration_schema.json

      "type": "array",
      "items": {
        "$ref": "#/$defs/MissionVariablePatternType"
      }
    },
-    "Required_Fields": {


Sorry - I meant to add a comment here with the rest. The use of "Fields" here is out of sync with the rest of the schema, which uses "Variables" throughout for the same concept. I wanted to make this more consistent, particularly with ExcludedScienceVariables.

flamingbear

Looks great, one question about dropping stuff from the sample config (but I'm pretty sure the answer is yet) and I'd rename CFOverrides now because just because we use it for that, it's not specific to CF.

flamingbear · 2024-09-10T21:36:04Z

config/1.0.0/earthdata_varinfo_configuration_schema.json

-        },
-        "required": ["Grid_Mapping_Dataset_Name", "grid_mapping_name"]
-      }
-    },


Are you losing any of that information? or was most of it copied from somewhere else in the first place? I'm ok with it all going since it can be added as a MetadataOverrides object.

flamingbear · 2024-09-10T21:40:56Z

config/1.0.0/sample_config_1.0.0.json

        ".*shot_number"
      ]
    }
  ],
-  "ProductEpochs": [
+  "CFOverrides": [


do it. MetadataOverrides

578faf8 🎉

flamingbear · 2024-09-10T21:45:55Z

config/1.0.0/sample_config_1.0.0.json

-        "Mission": "ICESat2"
+        "Mission": "SMAP",
+        "ShortNamePath": "SPL4.*"
      },
-      "Epoch": "2005-01-01T00:00:00.000000"
+      "Attributes": [
+        {
+          "Name": "grid_mapping",
+          "Value": "/EASE2_global_projection"
+        }
+      ]
    },
    {
      "Applicability": {
-        "Mission": "GEDI"
+        "Mission": "SMAP",
+        "ShortNamePath": "SPL4.*",
+        "VariablePattern": "/EASE2_global_projection"
      },
-      "Epoch": "2018-01-01T00:00:00.000000"
-    }
-  ],
-  "Grid_Mapping_Data": [
-    {
-      "Grid_Mapping_Dataset_Name": "EASE2_Global",
-      "grid_mapping_name": "lambert_cylindrical_equal_area",
-      "standard_parallel": 30.0,
-      "longitude_of_central_meridian": 0.0,
-      "false_easting": 0.0,
-      "false_northing": 0.0


So this is all just deleted, and not added because it wasn't used?

The ProductEpoch was entirely unused, so I just deleted it. I moved the content from Grid_Mapping_Data to the CFOverrides/MetadataOverrides section, though, just to provide examples for how that information would be included in the future. (In theory that sample configuration file should probably be slimmed down to a minimum number of examples, but I think that some of those rules were placed there so DAS has them somewhere)

owenlittlejohns

Quick summary of new changes:

Rename CFOverrides to MetadataOverrides.
Rename CFConfig.cf_overrides to CFConfig.metadata_overrides.
Rename CFConfig.get_cf_overrides to CFConfig.get_metadata_overrides.
Rename the cf_overrides property on all the Attribute/Group/Variable classes to be metadata_overrides. (That sounds like a lot, but it's mostly sorted via inheritance)
Review change logs (both schema and overall package) and reword/reorder things based on importance rather than the ordering in which they've been done on this feature branch.

eni-awowale

Tested with ATL06 and worked as expected!

TRT-556 - Remove ProductEpochs and Grid_Mapping_Data.

c6f2453

owenlittlejohns requested a review from flamingbear September 10, 2024 20:41

owenlittlejohns changed the base branch from main to TRT-552-configuration-file-schema-v2 September 10, 2024 20:42

owenlittlejohns commented Sep 10, 2024

View reviewed changes

owenlittlejohns requested a review from eni-awowale September 10, 2024 20:50

owenlittlejohns commented Sep 10, 2024

View reviewed changes

flamingbear approved these changes Sep 10, 2024

View reviewed changes

TRT-556 - Rename CFOverrides to MetadataOverrides.

578faf8

owenlittlejohns commented Sep 10, 2024

View reviewed changes

eni-awowale approved these changes Sep 11, 2024

View reviewed changes

owenlittlejohns merged commit 17c57a7 into TRT-552-configuration-file-schema-v2 Sep 11, 2024
3 checks passed

owenlittlejohns deleted the TRT-556-final-schema-clean-up branch September 11, 2024 15:20

owenlittlejohns mentioned this pull request Sep 11, 2024

TRT-552 - Implement configuration file schema v1.0.0 #39

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Trt 556 final schema clean up #38

Trt 556 final schema clean up #38

owenlittlejohns commented Sep 10, 2024 •

edited

Loading

owenlittlejohns Sep 10, 2024

flamingbear Sep 10, 2024

owenlittlejohns Sep 10, 2024 •

edited

Loading

owenlittlejohns Sep 10, 2024

owenlittlejohns Sep 10, 2024

flamingbear Sep 10, 2024

owenlittlejohns Sep 10, 2024

owenlittlejohns Sep 10, 2024

flamingbear left a comment

flamingbear Sep 10, 2024

flamingbear Sep 10, 2024

owenlittlejohns Sep 10, 2024

flamingbear Sep 10, 2024

owenlittlejohns Sep 10, 2024

owenlittlejohns left a comment •

edited

Loading

eni-awowale left a comment

		cls.namespace = 'namespace_string'
		cls.sample_config = 'config/0.0.1/sample_config_0.0.1.json'

Trt 556 final schema clean up #38

Trt 556 final schema clean up #38

Conversation

owenlittlejohns commented Sep 10, 2024 • edited Loading

Description

Jira Issue ID

Local Test Steps

PR Acceptance Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owenlittlejohns Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flamingbear left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owenlittlejohns left a comment • edited Loading

Choose a reason for hiding this comment

eni-awowale left a comment

Choose a reason for hiding this comment

owenlittlejohns commented Sep 10, 2024 •

edited

Loading

owenlittlejohns Sep 10, 2024 •

edited

Loading

owenlittlejohns left a comment •

edited

Loading