abi_l2_nc reader Key Error 'calibration' when trying to load Mask from fire Hot Spot

[frandorr]

Describe the bug
When attempting to load the "Mask" dataset from an L2 FDCF product using Satpy, the process fails with a KeyError related to the 'calibration' key during the dataset reading phase. This issue occurs despite following the typical procedure for loading datasets with Satpy. The expected behavior is for the mask dataset to be loaded without errors, but instead, an error is thrown indicating that the 'calibration' key is missing, which suggests there might be a missing or incorrect configuration in the Satpy reader or the dataset itself does not contain the expected information.

** Current workaround to load the Mask **
Modify abi_l2_nc.yaml file. Add a dummy calibration to the Mask part:

  fire_mask:
    name: Mask
    file_type: abi_l2_fdc
    file_key: Mask
    calibration: brightness_temperature # added this line

To Reproduce

from satpy import Scene

import fsspec

filename = "noaa-goes16/ABI-L2-FDCF/2019/003/02/OR_ABI-L2-FDCF-M3_G16_s20190030230362_e20190030241129_c20190030241241.nc"
the_files = fsspec.open_files("simplecache::s3://" + filename, s3={"anon": True})
print(the_files)

from satpy.readers import FSFile

fs_files = [FSFile(open_file) for open_file in the_files]

scn = Scene(filenames=fs_files, reader="abi_l2_nc")
scn.load(["Mask"])

Expected behavior
Load mask dataset from the l2 FDCF product.

Actual results

[DEBUG: 2024-03-21 20:47:27 : satpy.readers.yaml_reader] Reading ('/workspaces/sof-extractor/.venv/lib/python3.11/site-packages/satpy/etc/readers/abi_l2_nc.yaml',)
[DEBUG: 2024-03-21 20:47:27 : satpy.readers.yaml_reader] Assigning to abi_l2_nc: [<FSFile "noaa-goes16/ABI-L2-FDCF/2019/003/02/OR_ABI-L2-FDCF-M3_G16_s20190030230362_e20190030241129_c20190030241241.nc">]
<List of 1 OpenFile instances>
[DEBUG: 2024-03-21 20:47:27 : satpy.readers.abi_l2_nc] Reading in get_dataset Mask.
[WARNING: 2024-03-21 20:47:27 : satpy.readers.yaml_reader] Failed to load DataID(name='Mask', modifiers=()) from <NC_ABI_L2: 'noaa-goes16/ABI-L2-FDCF/2019/003/02/OR_ABI-L2-FDCF-M3_G16_s20190030230362_e20190030241129_c20190030241241.nc'>
Traceback (most recent call last):
  File "/workspaces/sof-extractor/.venv/lib/python3.11/site-packages/satpy/readers/yaml_reader.py", line 699, in _load_dataset
    projectable = fh.get_dataset(dsid, ds_info)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/sof-extractor/.venv/lib/python3.11/site-packages/satpy/readers/abi_l2_nc.py", line 46, in get_dataset
    if variable.attrs["units"] == "1" and key["calibration"] == "reflectance":
                                          ~~~^^^^^^^^^^^^^^^
KeyError: 'calibration'
[ERROR: 2024-03-21 20:47:27 : satpy.readers.yaml_reader] Could not load dataset 'DataID(name='Mask', modifiers=())': "Could not load DataID(name='Mask', modifiers=()) from any provided files"
Traceback (most recent call last):
  File "/workspaces/sof-extractor/.venv/lib/python3.11/site-packages/satpy/readers/yaml_reader.py", line 823, in _load_dataset_with_area
    ds = self._load_dataset_data(file_handlers, dsid, **kwargs)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/sof-extractor/.venv/lib/python3.11/site-packages/satpy/readers/yaml_reader.py", line 723, in _load_dataset_data
    proj = self._load_dataset(dsid, ds_info, file_handlers, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/workspaces/sof-extractor/.venv/lib/python3.11/site-packages/satpy/readers/yaml_reader.py", line 708, in _load_dataset
    raise KeyError(
KeyError: "Could not load DataID(name='Mask', modifiers=()) from any provided files"
[WARNING: 2024-03-21 20:47:27 : satpy.scene] The following datasets were not created and may require resampling to be generated: DataID(name='Mask', modifiers=())

Environment Info:

• OS: [e.g. Linux]
• Satpy Version: [e.g. 0.46.0]

[djhoese]

Thanks for filing a bug. This error surprises me. I believe the default DataID configuration is to include the calibration even if non-existent in the definition and is (I think) supposed to be set to None. @mraspaud That's what this means, right:

satpy/satpy/dataset/dataid.py

Lines 242 to 265 in 8527311

	default_id_keys_config = {"name": {
	"required": True,
	},
	"wavelength": {
	"type": WavelengthRange,
	},
	"resolution": {
	"transitive": False,
	},
	"calibration": {
	"enum": [
	"reflectance",
	"brightness_temperature",
	"radiance",
	"radiance_wavenumber",
	"counts"
	],
	"transitive": True,
	},
	"modifiers": {
	"default": ModifierTuple(),
	"type": ModifierTuple,
	},
	}

[frandorr]

Maybe a possible fix is changing key["calibration"] == "reflectance" to key.get("calibration") == "reflectance" in abi_l2_nc.py to avoid having problems if "calibration" key doesn't exist?

[djhoese]

The other question on my mind is why the tests aren't failing. The tests (if done correctly) should be doing the same kind of operations and shouldn't be patching/mocking any DataID stuff...hopefully.

[djhoese]

Ah the tests may not be testing this completely. Also I don't see this issue with other file types that I normally use like when I get the HT and TEMP variables since their units are not "1" and therefore don't go to the second half of the if statement.

[djhoese]

I was able to reproduce this with an ACM file and loading the BCM variable (which has units of 1). @frandorr Your fix makes sense to me. Do you think you could make a pull request and update the unit tests to make sure it works?

[frandorr]

Sure, I'll try to make it next week.

[djhoese]

@frandorr Any updates on your available time to make a PR for this?

[djhoese]

I created #2794 for this.

[frandorr]

Sorry, I didn't find the time to work on it

[djhoese]

No problem. It happens. And I just knew I didn't have time at that moment so was glad someone else might get to it. At least you found it. That's a major step.