DAS-2477 MaskFill raises a MissingCoordinateDataset when lat/lon datasets cannot be found in cf_config.coordinate_overrides or the dataset's 'coordinates' attribute.

[vutrannasa]

Description

MaskFill will raises a MissingCoordinateDataset when lat/lon datasets
cannot be found in cf_config.coordinate_overrides or the dataset's
'coordinates' attribute.

Jira Issue ID

https://bugs.earthdata.nasa.gov/browse/DAS-2477

Local Test Steps

Run the below command in SIT and verify fails

https://harmony.sit.earthdata.nasa.gov/C1260123843-EEDTEST/ogc-api-coverages/1.0.0/collections/h/coverage/rangeset?forceAsync=true&subset=lat(78%3A79)&subset=lon(14%3A17)&granuleId=G1260123848-EEDTEST&maxResults=1

Build local branch

$ git clone https://github.com/nasa/harmony-maskfill.git DAS-2477-nonetype
$ cd DAS-2477-nonetype
$ git checkout DAS-2477-nonetype

• Start Dockers

$ ./bin/build-image
$ ./bin/build-test
$ ./bin/run-test

Run the below command localhost and verify PASS

http://localhost:3000/C1260123843-EEDTEST/ogc-api-coverages/1.0.0/collections/h/coverage/rangeset?forceAsync=true&subset=lat(78%3A79)&subset=lon(14%3A17)&granuleId=G1260123848-EEDTEST&maxResults=1

Test TrajectorySubsetter_Regression.ipynb HOSS_Regression.ipynb in Harmony in a Box

% cd harmony
% vim .env (Verify .env has maskfill,hoss)
LOCALLY_DEPLOYED_SERVICES=maskfill,hoss

Load VScode and open harmony-regression-tests folder

• Select TrajectorySubsetter_Regression.ipynb
  -- Select Kernel: papermill-trajectory-subsetter
  -- In "Set the Harmony environment:" cell
  -- harmony_host_url = 'http://localhost:3000'
  -- Click "Run All" button
  -- Verify test runs without issues
  - 
• Select HOSS_Regression.ipynb
  -- In HOSS_Regression.ipynb
  -- Select Kernel: papermill-hoss
  -- In "Set the Harmony environment:" cell
  -- harmony_host_url = 'http://localhost:3000'
  -- Click "Run All" button
  -- Verify test runs without issues

Test using das-harmony-regression-test and verify

% cd das-harmony-regression-tests
% ./run_notebooks.sh --host local

PR Acceptance Checklist

• [ x] Jira ticket acceptance criteria met.
• [ x] CHANGELOG.md updated to include high level summary of PR changes.
• [ x] docker/service_version.txt updated if publishing a release.
• Fix version added to ticket - maskfill-X.Y.Z.
• [ x] Tests added/updated and passing.
• [ x] Documentation updated (if needed).

[vutrannasa]

latitude or longitude could be None so raise the same exception as line 137

[sudha-murthy]

This solution works. But should we be raising an exception when it is returning a None?
Shouldn't we just check for None in the function that is using it? We seem to be ignoring the exception raised and doing a 'pass' anyway?

[D-Auty]

Thanks for noticing this, Sudha - I too was fumbling about this construction, but didn't dig deep enough. The exception works, and is ok - we've seen plenty of examples of this style.

Oh - now I recall - this function is called in two contexts. One, the original implementation, needs to raise the exception and let it get handled up the call-stack - as it is "exceptional" in that context. The second case, does not need the exception, but is perhaps not enough to restructure things. I guess I am open to either approach.

[vutrannasa]

I think raising an exception here was a clearer implementation.

1. The calling code already handles MissingCoordinateDataset
2. It catches the issue at the source

Question: "We seem to be ignoring the exception raised and doing a 'pass' anyway?"
DAS-2399 originally aimed to check whether the coordinate dataset (lat/lon) consists entirely of fill values dataset_all_fill_value. If so, the dataset cannot be masked, so we log this condition and return early. Otherwise, we proceed with processing the dataset pass

[sudha-murthy]

I guess x and y dimension variables not being named a lat and lon seems normal ICESAT data to me and not an exception to be raised. Just a philosophical opinion.
But this works.

[sudha-murthy]

The build and test was fine.
The end to end test does not throw an exception.
Just need to add release tags in the change log. Looks like we missed it the last release as well

[sudha-murthy]

    Adding a check for None after line 150 in h5_maskfill.py accomplishes the same. That seems to have been the cause of the exception right?
     
    for coordinate in get_lon_lat_datasets(h5_dataset, cf_config):
            if coordinate is not None:
                if dataset_all_fill_value(coordinate,
                                      cf_config,
                                      logger,
                                      default_fill_value):
                    logger.info(f"The dataset {h5_dataset.name}'s coordinate "
                            f" {coordinate} contains only fill values, so "
                            f" the dataset cannot be masked")
                    return

[sudha-murthy]

I did not run the regression tests as that will be a separate PR.