In meta_yaml recipes section, dict_objects should be a list element

pangeo_forge_runner/feedstock.py

@@ -76,13 +76,22 @@ def parse_recipes(self):
         *Executes arbitrary code* defined in the feedstock recipes.
         """
         recipes = {}
-        if isinstance(self.meta.recipes, list):
-            for r in self.meta.recipes:
+        for r in self.meta.recipes:
+            assert any(
+                # MetaYaml schema validation of self.meta ensures that one of these two
+                # conditions is true, but just assert anyway, to make sure there are no
+                # edge cases that slip through the cracks.
+                [
+                    key_set == set(r.keys())
+                    for key_set in ({"id", "object"}, {"dict_object"})
+                ]
+            )
+            if {"id", "object"} == set(r.keys()):
                 recipes[r["id"]] = self._import(r["object"])
-        elif isinstance(self.meta.recipes, dict):
-            recipes = self._import(self.meta.recipes["dict_object"])
-        else:
-            raise ValueError("Could not parse recipes config in meta.yaml")
+            elif {"dict_object"} == set(r.keys()):
+                dict_object = self._import(r["dict_object"])
+                for k, v in dict_object.items():
+                    recipes[k] = v
 
         return recipes
 
@@ -94,19 +103,22 @@ def get_expanded_meta(self, drop_none=True) -> dict:
         'object' keys *may* be present, but not guaranteed.
         """
         meta_copy = deepcopy(self.meta)
-        if self.meta.recipes and "dict_object" in self.meta.recipes:
-            # We have a dict_object, so let's parse the recipes, and provide
-            # keep just the ids, discarding the values - as the values do not
+        if any(["dict_object" in r for r in self.meta.recipes]):
+            if not all(["dict_object" in r for r in self.meta.recipes]):
+                raise NotImplementedError(
+                    "Mixing explicit recipe objects and dict_objects in a "
+                    "single feedstock is not yet supported."
+                )
+            # We have at least one dict_object, so let's parse the recipes,
+            # keeping just the ids, discarding the values - as the values do not
             # actually serialize.
-            recipes = self.parse_recipes()
             meta_copy.recipes = [
-                # In place of the values, we add a placeholder string, so that the
+                # In place of dict values, we add a placeholder string, so that the
                 # re-assignment to the MetaYaml schema here will pass validation
                 # of the `recipes` field, which requires "id" and "object" fields.
                 {"id": k, "object": "DICT_VALUE_PLACEHOLDER"}
-                for k, _ in recipes.items()
+                for k, _ in self.parse_recipes().items()
             ]
-
         return (
             # the traitlets MetaYaml schema will give us empty containers
             # by default, but in most cases lets assume we don't want that

pangeo_forge_runner/meta_yaml.py

@@ -1,13 +1,17 @@
 import jsonschema
-from traitlets import Dict, HasTraits, List, TraitError, Unicode, Union, validate
+from traitlets import Dict, HasTraits, List, TraitError, Unicode, validate
 
 recipes_field_per_element_schema = {
     "type": "object",
     "properties": {
         "id": {"type": "string"},
         "object": {"type": "string"},
+        "dict_object": {"type": "string"},
     },
-    "required": ["id", "object"],
+    "oneOf": [
+        {"required": ["id", "object"]},
+        {"required": ["dict_object"]},
+    ],
 }
 
 
@@ -55,8 +59,8 @@ def _validate_recipes(self, proposal):
         Description of the dataset.
         """,
     )
-    recipes = Union(
-        [List(Dict()), Dict()],
+    recipes = List(
+        Dict(),
         help="""
         Specifies the deployable Python objects to run in the recipe module.
         If the recipes are assigned to their own Python variable names,
@@ -72,7 +76,7 @@ def _validate_recipes(self, proposal):
 
         ```yaml
         recipes:
-          dict_object: "recipe:name_of_recipes_dict"
+          - dict_object: "recipe:name_of_recipes_dict"
         ```
         """,
     )

tests/unit/test-recipes/dict-recipes/feedstock/meta.yaml

@@ -1,3 +1,3 @@
 title: "Test meta.yaml with list of recipes"
 recipes:
-  dict_object: 'recipe:recipes'
+  - dict_object: 'recipe:recipes'

tests/unit/test_feedstock.py

@@ -9,7 +9,7 @@
 yaml = YAML()
 
 
-@pytest.fixture(params=["recipe_object", "dict_object"])
+@pytest.fixture(params=["recipe_object", "dict_object", "both"])
 def tmp_feedstock(request, tmp_path_factory: pytest.TempPathFactory):
     tmpdir = tmp_path_factory.mktemp("feedstock")
     if request.param == "recipe_object":
@@ -32,7 +32,7 @@ class Recipe:
         meta_yaml = dedent(
             """\
         recipes:
-          dict_object: 'recipe:recipes'
+          - dict_object: 'recipe:recipes'
         """
         )
         recipe_py = dedent(
@@ -44,6 +44,25 @@ class Recipe:
         """
         )
 
+    elif request.param == "both":
+        meta_yaml = dedent(
+            """\
+        recipes:
+          - id: aws-noaa-sea-surface-temp-whoi
+            object: 'recipe:recipe'
+          - dict_object: 'recipe:recipes'
+        """
+        )
+        recipe_py = dedent(
+            """\
+        class Recipe:
+          pass
+
+        recipe = Recipe()
+        recipes = {"my_recipe": Recipe()}
+        """
+        )
+
     with open(tmpdir / "meta.yaml", mode="w") as f:
         f.write(meta_yaml)
     with open(tmpdir / "recipe.py", mode="w") as f:
@@ -59,21 +78,25 @@ def test_feedstock(tmp_feedstock):
     # so just check equality of the relevant trait (`.recipes`)
     assert f.meta.recipes == MetaYaml(**yaml.load(meta_yaml)).recipes
 
-    expanded_meta = f.get_expanded_meta()
     recipes = f.parse_recipes()
 
-    for recipe_metadata in expanded_meta["recipes"]:
-        # the recipe_object metadata  looks something like this:
-        #   {'recipes': [{'id': 'my_recipe', 'object': 'DICT_VALUE_PLACEHOLDER'}]}
-        # and the dict_object metadata looks like this:
-        #   {'recipes': [{'id': 'aws-noaa-sea-surface-temp-whoi', 'object': 'recipe:recipe'}]}
-        # both have an "id" field:
-        assert "id" in recipe_metadata
-        # but only the "recipe_object" has an "object" field:
-        if recipes_section_type == "recipe_object":
-            assert "object" in recipe_metadata
-        elif recipes_section_type == "dict_object":
-            assert recipe_metadata["object"] == "DICT_VALUE_PLACEHOLDER"
+    # the recipe_object metadata  looks something like this:
+    #   {'recipes': [{'id': 'my_recipe', 'object': 'DICT_VALUE_PLACEHOLDER'}]}
+    # and the dict_object metadata looks like this:
+    #   {'recipes': [{'id': 'aws-noaa-sea-surface-temp-whoi', 'object': 'recipe:recipe'}]}
+    if recipes_section_type == "recipe_object":
+        expanded_meta = f.get_expanded_meta()
+        assert expanded_meta["recipes"] == [
+            {"id": "aws-noaa-sea-surface-temp-whoi", "object": "recipe:recipe"},
+        ]
+    elif recipes_section_type == "dict_object":
+        expanded_meta = f.get_expanded_meta()
+        assert expanded_meta["recipes"] == [
+            {"id": "my_recipe", "object": "DICT_VALUE_PLACEHOLDER"},
+        ]
+    elif recipes_section_type == "both":
+        with pytest.raises(NotImplementedError):
+            _ = f.get_expanded_meta()
 
     for r in recipes.values():
         # the values of the recipes dict should all be python objects

tests/unit/test_meta_yaml.py

@@ -54,7 +54,7 @@ def valid_meta_yaml_dict_object(with_recipes_list: str) -> dict:
         dedent(
             """\
         recipes:
-          dict_object: 'recipe:recipes'
+          - dict_object: 'recipe:recipes'
         """
         ),
     )
@@ -111,6 +111,16 @@ def test_recipes_field_cannot_be_empty_container():
         _ = MetaYaml(recipes=[])
 
 
+def test_recipes_field_invalid_keys():
+    with pytest.raises(TraitError):
+        # "dict_object" key can't be used in combination with other keys
+        _ = MetaYaml(recipes={"id": "abcdefg", "dict_object": "abc:def"})
+    with pytest.raises(TraitError):
+        # the only valid keys are {"id", "object"} together,
+        # or "dict_object" alone. other keys are not allowed.
+        _ = MetaYaml(recipes={"some_other_key": "abc:def"})
+
+
 # TODO: In a future "strict" mode, ensure provenance fields are all provided.
 # --------------------------------------------------------------------------
 # @pytest.mark.parametrize(