Repository: dev-cafe/parselglossy

Issue:
Chain of dependencies
### Description

The following chain of dependencies currently does not work if neither `bar` nor `baz` is set:
```
keywords:
  - name: foo                                                             
    type: float                                                                  
    docstring: "Foo"
  - name: bar
    type: float
    default: "user['foo']"
    docstring: "Bar"
  - name: baz
    type: float
    default: "user['bar']"
    docstring: "Baz"
```
It seems `baz` is first set equal to the string `"user['foo']"` (which is a type mismatch), *before* `bar` is set to the value of `foo`. Related to issue #73.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/dev-cafe/parselglossy/pull/99?src=pr&el=h1) Report
> Merging [#99](https://codecov.io/gh/dev-cafe/parselglossy/pull/99?src=pr&el=desc) into [master](https://codecov.io/gh/dev-cafe/parselglossy/commit/55027bda3ff841955891a807dd8e59b923735e08&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `87.50%`.


bast: Thanks a lot for this! Looking at it ...
bast: > 2. Iterate over nodes in the `DiGraph` and check whether a keyword in the list of keywords has the same name as a node. If yes, deep copy them to a `shuffle` list and remove them from the original list. This effectively splits the list of keywords for the top-level section into two parts: "spectator" keywords, _i.e._ those that are independent of other keywords in the list, and "dependent" keywords (in the `shuffle` list). Once we're done we can append the `shuffle` list to the "spectator" list. Given that we loop over nodes in the `DiGraph` the `shuffle` list is already in the desired order.

I was expecting that we would solve this with some of the sort functions from networkx now that we know that the graph is sortable. But this is rather solved by deepcopy. I am still trying to wrap my head around it: how do we make sure that the deepcopies are processed in the right order? Looking at the tests it seems to work though so it's probably correct.

Patch:
diff --git a/parselglossy/api.py b/parselglossy/api.py
index 99ea3d4..ae507bf 100644
--- a/parselglossy/api.py
+++ b/parselglossy/api.py
@@ -174,7 +174,7 @@ def generate(
     stencil = read_yaml_file(template_)
 
     # Check if the template is valid
-    is_template_valid(stencil)
+    stencil = is_template_valid(stencil)
 
     with (where_ / "api.py").open("w") as f:
         f.write(generation.api_py(stencil))
@@ -330,7 +330,7 @@ def document(
     template = path_resolver(template)
     stencil = read_yaml_file(template)
 
-    is_template_valid(stencil)
+    stencil = is_template_valid(stencil)
 
     docs = documentation_generator(stencil, header=header)
 
diff --git a/parselglossy/generation.py b/parselglossy/generation.py
index 0560fdf..1183fc5 100644
--- a/parselglossy/generation.py
+++ b/parselglossy/generation.py
@@ -159,7 +159,7 @@ from typing import Optional, Union
 
 from .plumbing import lexer
 from .plumbing.utils import JSONDict, as_complex, path_resolver, copier
-from .plumbing.validation import is_template_valid, validate_from_dicts
+from .plumbing.validation import validate_from_dicts
 
 
 def lex(
diff --git a/parselglossy/utils.py b/parselglossy/utils.py
index f87126f..f558981 100644
--- a/parselglossy/utils.py
+++ b/parselglossy/utils.py
@@ -33,7 +33,7 @@ import json
 from functools import reduce
 from pathlib import Path
 from shutil import copy
-from typing import Any, Dict, List, Tuple, Union
+from typing import Any, Dict, List, Optional, Tuple, Union
 
 JSONDict = Dict[str, Any]
 
@@ -69,7 +69,7 @@ def location_in_dict(*, address: Tuple, dict_name: str = "user") -> str:
     -------
     where : str
     """
-    return reduce(lambda x, y: x + "['{}']".format(y), address, "user")
+    return reduce(lambda x, y: x + f"['{y}']", address, "user")
 
 
 def path_resolver(f: Union[str, Path], *, touch: bool = True) -> Path:
@@ -169,3 +169,44 @@ def dict_to_list(d: JSONDict) -> List[Any]:
         else:
             nested_list.append([k, v])
     return nested_list
+
+
+def nested_get(d: JSONDict, *ks: str) -> Optional[Any]:
+    """Get value from a nested dictionary.
+
+    Parameters
+    ----------
+    d : JSONDict
+    ks : str
+
+    Returns
+    -------
+    v : Optional[Any]
+
+    Notes
+    -----
+    Adapted from: https://stackoverflow.com/a/40675868/2528668
+    """
+
+    def _func(x: JSONDict, k: str) -> Optional[JSONDict]:
+        return x.get(k, None) if isinstance(x, dict) else None
+
+    return reduce(_func, ks, d)  # type: ignore
+
+
+def nested_set(d: JSONDict, ks: Tuple[Any, ...], v: Any) -> None:
+    """Set value in nested dictionary.
+
+    Parameters
+    ----------
+    d : JSONDict
+    ks : Tuple[str]
+    v : Any
+
+    Notes
+    -----
+    Adapted from: https://stackoverflow.com/a/13688108/2528668
+    """
+    for k in ks[:-1]:
+        d = d.setdefault(k, {})
+    d[ks[-1]] = v
diff --git a/parselglossy/validation.py b/parselglossy/validation.py
index ad4f347..b80f0c4 100644
--- a/parselglossy/validation.py
+++ b/parselglossy/validation.py
@@ -41,6 +41,7 @@ from .validation_plumbing import (
     _rec_fix_defaults,
     _rec_is_template_valid,
     _rec_merge_ours,
+    _reorder_template,
 )
 from .views import view_by_default, view_by_predicates, view_by_type
 
@@ -70,7 +71,7 @@ def validate_from_dicts(
     ------
     :exc:`ParselglossyError`
     """
-    is_template_valid(template)
+    template = is_template_valid(template)
     stencil = view_by_default(template)
     types = view_by_type(template)
     predicates = view_by_predicates(template)
@@ -87,7 +88,7 @@ def validate_from_dicts(
     return fr
 
 
-def is_template_valid(template: JSONDict) -> None:
+def is_template_valid(template: JSONDict) -> JSONDict:
     """Checks a template ``dict`` is well-formed.
 
     A template ``dict`` is well-formed if:
@@ -105,17 +106,23 @@ def is_template_valid(template: JSONDict) -> None:
 
     * All sections have a non-empty docstring.
 
+    * Reorders keywords according to their dependencies.
+
     Parameters
     ----------
     template : JSONDict
 
+    Returns
+    -------
+    ordered : JSONDict
+
     Raises
     ------
     :exc:`ParselglossyError`
 
     Notes
     -----
-    This is porcelain over the recursive :func:`rec_is_template_valid`.
+    This is porcelain over the recursive :func:`_rec_is_template_valid`.
     """
 
     errors = _rec_is_template_valid(template)
@@ -124,6 +131,8 @@ def is_template_valid(template: JSONDict) -> None:
     if errors:
         msg = collate_errors(when="checking the template", errors=errors)
         raise ParselglossyError(msg)
+    else:
+        return _reorder_template(template)
 
 
 def merge_ours(*, theirs: JSONDict, ours: JSONDict) -> JSONDict:
@@ -150,7 +159,7 @@ def merge_ours(*, theirs: JSONDict, ours: JSONDict) -> JSONDict:
     validated, input dictionary by using default values where these are not
     overridden by user input, hence the naming "ours" for the merge strategy.
 
-    This is porcelain over the recursive function :func:`rec_merge_ours`.
+    This is porcelain over the recursive function :func:`_rec_merge_ours`.
     """
     outgoing, errors = _rec_merge_ours(theirs=theirs, ours=ours)
 
@@ -208,7 +217,7 @@ def fix_defaults(incoming: JSONDict, *, types: JSONDict) -> JSONDict:
     3. If even this check was unsuccessful, types really were unmatched. We
     report the error and move on.
 
-    This is porcelain over recursive function :func:`rec_fix_defaults`.
+    This is porcelain over recursive function :func:`_rec_fix_defaults`.
     """
 
     outgoing, errors = _rec_fix_defaults(incoming, types=types)
@@ -236,7 +245,7 @@ def check_predicates(incoming: JSONDict, *, predicates: JSONDict) -> None:
 
     Notes
     -----
-    This is porcelain over recursive function :func:`rec_check_predicates`.
+    This is porcelain over recursive function :func:`_rec_check_predicates`.
     """
 
     errors = _rec_check_predicates(incoming, predicates=predicates)
diff --git a/parselglossy/validation_plumbing.py b/parselglossy/validation_plumbing.py
index 722815f..38b6eee 100644
--- a/parselglossy/validation_plumbing.py
+++ b/parselglossy/validation_plumbing.py
@@ -30,14 +30,15 @@
 """Plumbing functions powering our validation facilities."""
 
 import re
+from copy import deepcopy
 from typing import Any, List, Optional, Tuple
 
 import networkx as nx
 
 from .exceptions import Error
 from .types import allowed_types, type_fixers, type_matches
-from .utils import JSONDict, location_in_dict
-from .views import view_by_default
+from .utils import JSONDict, location_in_dict, nested_set
+from .views import view_by_default, view_by_default_keywords
 
 
 def _rec_is_template_valid(template: JSONDict, *, address: Tuple = ()) -> List[Error]:
@@ -149,6 +150,58 @@ def _keywords_default_dependencies(
     return dependencies
 
 
+def _reorder_template(template: JSONDict) -> JSONDict:
+    """Reorder template according to direct graph of keyword dependencies
+
+    Parameters
+    ----------
+    template : JSONDict
+
+    Returns
+    -------
+    ordered : JSONDict
+
+    Notes
+    -----
+    This function reorders the template in place.
+
+    Warnings
+    --------
+    We are assuming that there are no dependency cycles in the template. Call
+    this function **after** :func:`_check_cyclic_defaults`.
+    """
+
+    ordered = deepcopy(template)
+    _rec_reoder_template(ordered)
+    return ordered
+
+
+def _rec_reoder_template(template: JSONDict) -> None:
+    """Recursive backend for :func:`_reorder_template`.
+
+    Parameters
+    ----------
+    template : JSONDict
+    """
+    keywords = template["keywords"] if "keywords" in template.keys() else []
+    kw_stencil = view_by_default_keywords(keywords)
+    deps = _sections_default_dependencies(kw_stencil)
+    deps_hashable = [(tuple(_from), tuple(_to)) for (_from, _to) in deps]
+    nodes = reversed([x[-1] for x in nx.DiGraph(deps_hashable)])
+
+    shuffle = []
+    for node in nodes:
+        for x in keywords:
+            if x["name"] == node:
+                shuffle.append(deepcopy(x))
+                keywords.remove(x)
+    keywords.extend(shuffle)
+
+    sections = template["sections"] if "sections" in template.keys() else []
+    for s in sections:
+        _rec_reoder_template(s)
+
+
 def _rec_merge_ours(
     *, theirs: JSONDict, ours: JSONDict, address: Tuple = ()
 ) -> Tuple[JSONDict, List[Error]]:
@@ -181,7 +234,7 @@ def _rec_merge_ours(
     if difference != set():
         for k in difference:
             what = "section" if isinstance(ours[k], dict) else "keyword"
-            errors.append(Error(message="Found unexpected {}: '{}'.".format(what, k)))
+            errors.append(Error(message=f"Found unexpected {what}: '{k}'."))
 
     for k, v in theirs.items():
         if k not in ours.keys():
@@ -189,7 +242,7 @@ def _rec_merge_ours(
                 outgoing[k] = theirs[k]
             else:
                 outgoing[k] = None
-                msg = "Keyword '{0}' is required but has no value.".format(k)
+                msg = f"Keyword '{k}' is required but has no value."
                 errors.append(Error((address + (k,)), msg))
         elif not isinstance(v, dict):
             outgoing[k] = ours[k]
@@ -261,12 +314,12 @@ def _rec_fix_defaults(
     report the error and move on.
     """
 
+    if start_dict is None:
+        start_dict = deepcopy(incoming)
+
     outgoing = {}
     errors = []
 
-    if start_dict is None:
-        start_dict = incoming
-
     for k, v in incoming.items():
         if not isinstance(v, dict):
             t = types[k]
@@ -288,13 +341,16 @@ def _rec_fix_defaults(
                     actual = (
                         type(v).__name__
                         if type(v) is not list
-                        else "List[{}]".format(", ".join([type(x).__name__ for x in v]))
-                    )
-                    msg = "Actual ({0}) and declared ({1}) types do not match.".format(
-                        actual, t
+                        else f"List[{', '.join([type(x).__name__ for x in v])}]"
                     )
+                    msg = f"Actual ({actual}) and declared ({t}) types do not match."
             if msg != "":
                 errors.append(Error(address + (k,), msg))
+            else:
+                # Update start_dict.
+                # This is so that multiple dependent defaults ("chains") behave
+                # correctly. See #76 on GitHub
+                nested_set(start_dict, address + (k,), outgoing[k])
         else:
             outgoing[k], errs = _rec_fix_defaults(
                 incoming=v,
@@ -376,24 +432,23 @@ def run_predicate(predicate: str, where: str, user: JSONDict) -> Tuple[str, bool
     """
 
     p = predicate.replace("value", where)
-    postfix = "in closure '{}'.".format(predicate)
 
     try:
         msg = ""
-        success = eval("lambda user: {}".format(p))(user)
+        success = eval(f"lambda user: {p}")(user)
         if not success:
-            msg = "Predicate '{}' not satisfied.".format(predicate)
+            msg = f"Predicate '{predicate}' not satisfied."
     except KeyError as e:
-        msg = "KeyError {} {:s}".format(e, postfix)
+        msg = f"KeyError {e} in closure '{predicate}'."
         success = False
     except SyntaxError as e:
-        msg = "SyntaxError {} {:s}".format(e, postfix)
+        msg = f"SyntaxError {e} in closure '{predicate}'."
         success = False
     except TypeError as e:
-        msg = "TypeError {} {:s}".format(e, postfix)
+        msg = f"TypeError {e} in closure '{predicate}'."
         success = False
     except NameError as e:
-        msg = "NameError {} {:s}".format(e, postfix)
+        msg = f"NameError {e} in closure '{predicate}'."
         success = False
 
     return msg, success
@@ -432,24 +487,21 @@ def run_callable(f: str, d: JSONDict, *, t: str) -> Tuple[str, Optional[Any]]:
     any point.
     """
 
-    closure = "lambda user: {}"
-    postfix = "in closure '{}'.".format(f)
-
     try:
         msg = ""
-        result = eval(closure.format(f))(d)
+        result = eval(f"lambda user: {f}")(d)
         result = type_fixers[t](result)
     except KeyError as e:
-        msg = "KeyError {} {:s}".format(e, postfix)
+        msg = f"KeyError {e} in closure '{f}'"
         result = None
     except SyntaxError as e:
-        msg = "SyntaxError {} {:s}".format(e, postfix)
+        msg = f"SyntaxError {e}  in closure '{f}'"
         result = None
     except TypeError as e:
-        msg = "TypeError {} {:s}".format(e, postfix)
+        msg = f"TypeError {e}  in closure '{f}'"
         result = None
     except NameError as e:
-        msg = "NameError {} {:s}".format(e, postfix)
+        msg = f"NameError {e} in closure '{f}'"
         result = None
 
     return msg, result
diff --git a/parselglossy/views.py b/parselglossy/views.py
index 276a099..3e8c5f9 100644
--- a/parselglossy/views.py
+++ b/parselglossy/views.py
@@ -29,7 +29,7 @@
 
 """Tools to extract views of dictionaries."""
 
-from typing import Any, Callable, Optional, Type
+from typing import Any, Callable, List, Optional, Type
 
 from .exceptions import ParselglossyError
 from .utils import JSONDict
@@ -65,6 +65,21 @@ def view_by_default(d: JSONDict) -> JSONDict:
     return view_by("default", d)
 
 
+def view_by_default_keywords(keywords: List[JSONDict]) -> JSONDict:
+    """View by defaults only for lists of keywords.
+
+    Parameters
+    ----------
+    keywords: List[JSONDict]
+
+    Returns
+    -------
+    outgoing: JSONDict
+       A dictionary with a view by defaults for the keywords.
+    """
+    return {v["name"]: v["default"] if "default" in v else None for v in keywords}
+
+
 def view_by_docstring(d: JSONDict) -> JSONDict:
     """Partial application of :func:`view_by` for docstrings.
 
diff --git a/parselglossy/yaml_utils.py b/parselglossy/yaml_utils.py
index fa52bd4..063baa5 100644
--- a/parselglossy/yaml_utils.py
+++ b/parselglossy/yaml_utils.py
@@ -28,18 +28,19 @@
 #
 
 from pathlib import Path
+from typing import Union
 
 import yaml
 
 from .utils import JSONDict
 
 
-def read_yaml_file(file_name: Path) -> JSONDict:
+def read_yaml_file(file_name: Union[str, Path]) -> JSONDict:
     """Reads a YAML file and returns it as a dictionary.
 
     Parameters
     ----------
-    file_name: Path
+    file_name: Union[str, Path]
         Path object for the YAML file.
 
     Returns
@@ -47,6 +48,9 @@ def read_yaml_file(file_name: Path) -> JSONDict:
     d: JSONDict
         A dictionary with the contents of the YAML file.
     """
+
+    file_name = Path(file_name) if isinstance(file_name, str) else file_name
+
     with file_name.open("r") as f:
         try:
             d = yaml.safe_load(f)
diff --git a/pyproject.toml b/pyproject.toml
index e283f1e..e0644be 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -8,6 +8,23 @@ line_length = 88
 [tool.black]
 exclude = 'pyparsing.py'
 
+[tool.pytest.ini_options]
+addopts = "-rws"
+testpaths = [
+    "tests",
+]
+norecursedirs = [
+  "env",
+  ".direnv",
+  "venv",
+  ".env",
+  ".venv",
+  "docs",
+  ".eggs",
+  ".git",
+]
+collect_ignore = ["setup.py"]
+
 [build-system]
 requires = ["flit_core >=2,<4"]
 build-backend = "flit_core.buildapi"


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 279c314..02aeeac 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -2,7 +2,14 @@ name: Test parselglossy
 
 on:
   push:
+    branches:
+      - master
   pull_request:
+    branches:
+      - master
+  release:
+    types:
+      - created
 
 jobs:
   build:
diff --git a/tests/test_check_template.py b/tests/test_check_template.py
index c011b23..360571b 100644
--- a/tests/test_check_template.py
+++ b/tests/test_check_template.py
@@ -277,4 +277,4 @@ check_template_data = [
 )
 def test_check_template(template, raises):
     with raises:
-        is_template_valid(template)
+        template = is_template_valid(template)
diff --git a/tests/test_validation.py b/tests/test_validation.py
index cb90944..41ae4b7 100644
--- a/tests/test_validation.py
+++ b/tests/test_validation.py
@@ -28,7 +28,6 @@
 #
 
 import json
-import re
 from contextlib import ExitStack as does_not_raise
 from pathlib import Path
 from typing import List
@@ -223,6 +222,20 @@ validation_data = (
             {"foobar": True, "foo": {"bar": False}},
             [""],
         ),
+        (
+            "overall",
+            "input_ordering.yml",
+            "ordering.yml",
+            None,
+            {
+                "flavor": 2.0,
+                "color": 2.0,
+                "charge": 2.0,
+                "spectator": "nothing to see",
+                "adorable_kitten": {"flavor": 1.0, "color": 1.5, "charge": 1.0,},
+            },
+            [""],
+        ),
     ]
     + template_errors_data
     + input_errors_data
diff --git a/tests/validation/overall/input_ordering.yml b/tests/validation/overall/input_ordering.yml
new file mode 100644
index 0000000..fc02fc0
--- /dev/null
+++ b/tests/validation/overall/input_ordering.yml
@@ -0,0 +1,4 @@
+flavor: 2.0
+spectator: "nothing to see"
+adorable_kitten:
+  flavor: 1.0
diff --git a/tests/validation/overall/ordering.yml b/tests/validation/overall/ordering.yml
new file mode 100644
index 0000000..c5ecf6a
--- /dev/null
+++ b/tests/validation/overall/ordering.yml
@@ -0,0 +1,32 @@
+keywords:
+  - name: flavor
+    type: float
+    docstring: "Foo"
+  - name: color
+    type: float
+    default: "user['charge']"
+    docstring: "Baz"
+  - name: charge
+    type: float
+    default: "user['flavor']"
+    docstring: "Bar"
+  - name: spectator
+    type: str
+    docstring: |
+      A spectator keyword
+sections:
+  - name: adorable_kitten
+    docstring: |
+      A section in our input tree
+    keywords:
+      - name: flavor
+        type: float
+        docstring: "Foo"
+      - name: color
+        type: float
+        default: "1.5 * user['adorable_kitten']['charge']"
+        docstring: "Baz"
+      - name: charge
+        type: float
+        default: "user['adorable_kitten']['flavor']"
+        docstring: "Bar"