Reference optional composite dependencies by name

[pnuu]

Code Sample, a minimal, complete, and verifiable piece of code

Sample compositor config

  my_composite:
    compositor: !!python/name:my_composites.MyCompositor
    prerequisites:
      - 10.8
    optional_prerequisites:
      - solar_zenith_angle
      - 0.6

The compositor for above config

class MyCompositor(CompositeBase):

    def __call__(self, projectables=None, optional_datasets=None, **info):
        data_1 = projectables[0].copy()
        sza, data_2 = optional_datasets
        # If data 2 not available, use plain IR image
        if data_2 is None:
            return Dataset(data_1, **info.copy())
        # If SZA not available, calculate them
        if sza is None:
            from pyorbital.astronomy import sun_zenith_angle
            sza = sun_zenith_angle(data_1.info["start_time"], *data_1.info["area"].get_lonlats())
        # Use data 2 where Sun is high enough
        data_1[sza < 90] = data_2[sza < 90]
        return Dataset(data_1, **info.copy())

Test script

from satpy import Scene
import glob
# Use data that does no yield solar_zenith_angle, and/or remove 0.6 um data
fnames = glob.glob("/path/to/data/*")
glbl = Scene(filenames=fnames)
glbl.load(['my_composite'])

Problem description

If, when creating the above composite, either of the optional datasets are unavailable, the optional_dataset is a list of length one, and thus the unpacking fails. Also, the missing dataset can't be determined as the order is critical.

Expected Output

1. With all data available: composite with night-side data from dataset 1 and day-side from dataset 2
2. With SZA unavailable: composite with night-side data from dataset 1 and day-side from dataset 2
3. With dataset 1 missing: composite with everything from dataset 1
4. With SZA and dataset 2 missing: composite with everything from dataset 1

Actual Result, Traceback if applicable

For the above list:

1. works as planned
2. ValueError: need more than 1 values to unpack
3. ValueError: need more than 1 values to unpack
4. ValueError: need more than 0 values to unpack

Versions of Python, package at hand and relevant dependencies

Current SatPy develop branch

[pnuu]

I see two (or three) solutions.

1. place None to optional_datasets where the respective dataset is missing (as assumed in the above example)
2. Use a dictionary to pass the optional datasets
3. Use dictionaries for both projectables and optional_datasets and use "standardized" naming in the compositors

[djhoese]

I'm not sure solutions 2 or 3 would work since we use DatasetIDs for unique identification and there is a chance that a composite could have two datasets with the same name. Not to mention some generic composites only use the wavelength so using the name of the band wouldn't happen.

So for me that only leaves option 1. The other thing to note is you could check the length of the optional datasets and then look at the datasets metadata to get this to work.

[pnuu]

In the case of generic compositors, checking metadata won't help, as the compositor doesn't know what it should receive. Unless it also receives the contents of the config. But solution 1. should work anyway. In solutions 2. and 3. I meant that the dictionary keys would be descriptive of the usage in the compositer. For example r, g and b, or day_data, night_data and solar_zenith_angle.

[pnuu]

And clarify more, the config would look something like this (for option 3):

  my_composite:
    compositor: !!python/name:my_composites.MyCompositor
    prerequisites:
      night_data: 10.8
    optional_prerequisites:
      solar_zenith_angle: solar_zenith_angle
      day_data: 0.6

And compositor something like this (didn't think this completely through, so optimization is in order):

class MyCompositor(CompositeBase):
    def __call__(self, projectables=None, optional_datasets=None, **info):
        data_1 = projectables['night_data'].copy()
        try:
            data_2 = optional_datasets['day_data']
        except (KeyError, TypeError):
            return Dataset(data_1, **info.copy())
        try:
            sza = optional_datasets['solar_zenith_angle']
        except KeyError:
            from pyorbital.astronomy import sun_zenith_angle
            sza = sun_zenith_angle(data_1.info["start_time"], *data_1.info["area"].get_lonlats())
        data_1[sza < 90] = data_2[sza < 90]
        return Dataset(data_1, **info.copy())

[pnuu]

@mraspaud suggested this, using the same configuration. Evidently more clear:

def __call__(self, night_data, day_data=None, solar_zenith_angle=None, **info):

[mraspaud]

I propose that we explicit the name of the arguments if the __call__ prototype, eg:

class MyCompositor(CompositeBase):
    def __call__(self, night_data, day_data=None, solar_zenith_angle=None, **info):
        data_1 = night_data.copy()
        try:
            data_2 = day_data.copy()
        except AttributeError:
            return Dataset(data_1, **info.copy())
        try:
            sza = solar_zenith_angle.copy()
        except AttributeError:
            from pyorbital.astronomy import sun_zenith_angle
            sza = sun_zenith_angle(data_1.info["start_time"], *data_1.info["area"].get_lonlats())
        data_1[sza < 90] = data_2[sza < 90]
        return Dataset(data_1, **info.copy())

On the caller side, that would become something along the lines of compositor(*prerequisites, **optional_prerequisites)

[djhoese]

Oh very interesting. We could have it just resemble the YAML file so people could do lists or dicts. Not sure if that is too many ways to do something. Either way, is there any reason that the prerequisites couldn't be a dict? Probably more confusing to allow that I guess.

[djhoese]

Late night thought: what about required and optional in the YAML?

[pnuu]

@davidh-ssec Yes, those sound good, as long as the same naming is used in the compositors for developers' benefit :-)

I'm tempted to suggest that we'd swap over to using only dictionaries and deprecate the list-based composite configs/compositors. The YAML files would be then self-describing what the datasets are.

[djhoese]

Sounds good to me. I was thinking it might be a pain to have to switch all the compositors over, but now that we realize this obvious feature/interface it doesn't make sense to do the lists. At least not that I can think of.

[djhoese]

@mraspaud I'm moving this to 0.10 since we are closer to 0.9 now and I don't want this to break everything that users have done up to now. Feel free to disagree and change the milestone back.