Warn on unused arguments

buildingmotif/database/table_connection.py

@@ -406,7 +406,7 @@ def add_template_dependency(
                 f"In {templ.name} the values of the bindings to {dep.name} must correspond to the "
                 "parameters in the dependant template."
                 f"Dependency {dep.name} refers to params {set(args.values()).difference(params)} "
-                f"that do not appear in template {templ.name}"
+                f"that do not appear in template {templ.name} ({params})"
             )
         # do the same for the dependency
         graph = self.bm.graph_connection.get_graph(dep.body_id)

buildingmotif/dataclasses/library.py

@@ -18,10 +18,13 @@
 from buildingmotif.database.tables import DBLibrary, DBTemplate
 from buildingmotif.dataclasses.shape_collection import ShapeCollection
 from buildingmotif.dataclasses.template import Template
-from buildingmotif.namespaces import XSD
 from buildingmotif.schemas import validate_libraries_yaml
 from buildingmotif.template_compilation import compile_template_spec
-from buildingmotif.utils import get_ontology_files, get_template_parts_from_shape
+from buildingmotif.utils import (
+    get_ontology_files,
+    get_template_parts_from_shape,
+    skip_uri,
+)
 
 if TYPE_CHECKING:
     from buildingmotif import BuildingMOTIF
@@ -386,12 +389,8 @@ def _resolve_template_dependencies(
                     if dep["template"] in template_id_lookup:
                         dependee = Template.load(template_id_lookup[dep["template"]])
                         template.add_dependency(dependee, dep["args"])
-                    # Now that we have all the templates, we can populate the dependencies.
-                    # IGNORES missing XSD imports --- there is really no reason to import the XSD
-                    # ontology because the handling is baked into the software processing the RDF
-                    # graph. Thus, XSD URIs will always yield import warnings. This is noisy, so we
-                    # suppress them.
-                    elif not dep["template"].startswith(XSD):
+                    # check documentation for skip_uri for what URIs get skipped
+                    elif not skip_uri(dep["template"]):
                         logging.warn(
                             f"Warning: could not find dependee {dep['template']}"
                         )

buildingmotif/dataclasses/template.py

@@ -1,3 +1,4 @@
+import warnings
 from collections import Counter
 from dataclasses import dataclass
 from itertools import chain
@@ -122,7 +123,7 @@ def remove_dependency(self, dependency: "Template") -> None:
     @property
     def all_parameters(self) -> Set[str]:
         """The set of all parameters used in this template *including* its
-        dependencies.
+        dependencies. Includes optional parameters.
 
         :return: set of parameters *with* dependencies
         :rtype: Set[str]
@@ -138,7 +139,7 @@ def all_parameters(self) -> Set[str]:
     @property
     def parameters(self) -> Set[str]:
         """The set of all parameters used in this template *excluding* its
-        dependencies.
+        dependencies. Includes optional parameters.
 
         :return: set of parameters *without* dependencies
         :rtype: Set[str]
@@ -150,7 +151,8 @@ def parameters(self) -> Set[str]:
 
     @property
     def dependency_parameters(self) -> Set[str]:
-        """The set of all parameters used in this demplate's dependencies.
+        """The set of all parameters used in this template's dependencies, including
+        optional parameters.
 
         :return: set of parameters used in dependencies
         :rtype: Set[str]
@@ -230,7 +232,8 @@ def inline_dependencies(self) -> "Template":
         if not self.get_dependencies():
             return templ
 
-        # start with this template's parameters; if there is
+        # start with this template's parameters; this recurses into each
+        # dependency to inline dependencies
         for dep in self.get_dependencies():
             # get the inlined version of the dependency
             deptempl = dep.template.inline_dependencies()
@@ -256,15 +259,22 @@ def inline_dependencies(self) -> "Template":
             replace_nodes(
                 deptempl.body, {PARAM[k]: PARAM[v] for k, v in rename_params.items()}
             )
-            # be sure to rename optional arguments too
-            unused_optional_args = set(deptempl.optional_args) - set(dep.args.keys())
-            dep_optional_args = [
-                rename_params.get(arg, arg) for arg in unused_optional_args
-            ]
 
-            # append into the dependant body
+            # figure out which of deptempl's parameters are encoded as 'optional' by the
+            # parent (depending) template
+            deptempl_opt_args = deptempl.parameters.intersection(templ.optional_args)
+            # if the 'name' of the deptempl is optional, then all the arguments inside deptempl
+            # become optional
+            if rename_params["name"] in deptempl_opt_args:
+                # mark all of deptempl's parameters as optional
+                templ.optional_args += deptempl.parameters
+            else:
+                # otherwise, only add the parameters that are explicitly
+                # marked as optional *and* appear in this dependency
+                templ.optional_args += deptempl_opt_args
+
+            # append the inlined template into the parent's body
             templ.body += deptempl.body
-            templ.optional_args += dep_optional_args
 
         return templ
 
@@ -273,6 +283,7 @@ def evaluate(
         bindings: Dict[str, Node],
         namespaces: Optional[Dict[str, rdflib.Namespace]] = None,
         require_optional_args: bool = False,
+        warn_unused: bool = True,
     ) -> Union["Template", rdflib.Graph]:
         """Evaluate the template with the provided bindings.
 
@@ -292,13 +303,29 @@ def evaluate(
         :param require_optional_args: whether to require all optional arguments
             to be bound, defaults to False
         :type require_optional_args: bool
+        :param warn_unused: if True, print a warning if there were any parameters left
+            unbound during evaluation. If 'require_optional_args' is True,
+            then this will consider optional parameters when producing the warning;
+            otherwise, optional paramters will be ignored; defaults to True
+        :type warn_unused: bool
         :return: either a template or a graph, depending on whether all
             parameters were provided
         :rtype: Union[Template, rdflib.Graph]
         """
         templ = self.in_memory_copy()
+        # put all of the parameter names into the PARAM namespace so they can be
+        # directly subsituted in the template body
         uri_bindings: Dict[Node, Node] = {PARAM[k]: v for k, v in bindings.items()}
+        # replace the param:<name> URIs in the template body with the bindings
+        # provided in the call to evaluate()
         replace_nodes(templ.body, uri_bindings)
+        leftover_params = (
+            templ.parameters.difference(bindings.keys())
+            if not require_optional_args
+            else (templ.parameters.union(self.optional_args)).difference(
+                bindings.keys()
+            )
+        )
         # true if all parameters are now bound or only optional args are unbound
         if len(templ.parameters) == 0 or (
             not require_optional_args and templ.parameters == set(self.optional_args)
@@ -313,6 +340,10 @@ def evaluate(
                 for arg in unbound_optional_args:
                     remove_triples_with_node(templ.body, PARAM[arg])
             return templ.body
+        if len(leftover_params) > 0 and warn_unused:
+            warnings.warn(
+                f"Parameters \"{', '.join(leftover_params)}\" were not provided during evaluation"
+            )
         return templ
 
     def fill(self, ns: rdflib.Namespace) -> Tuple[Dict[str, Node], rdflib.Graph]:

buildingmotif/utils.py

@@ -11,7 +11,7 @@
 from rdflib.paths import ZeroOrOne
 from rdflib.term import Node
 
-from buildingmotif.namespaces import OWL, PARAM, RDF, SH, bind_prefixes
+from buildingmotif.namespaces import OWL, PARAM, RDF, SH, XSD, bind_prefixes
 
 if TYPE_CHECKING:
     from buildingmotif.dataclasses import Template
@@ -29,6 +29,14 @@ def _gensym(prefix: str = "p") -> URIRef:
     return PARAM[f"{prefix}{_gensym_counter}"]
 
 
+def _param_name(param: URIRef) -> str:
+    """
+    Returns the name of the param w/o the prefix
+    """
+    assert param.startswith(PARAM)
+    return param[len(PARAM) :]
+
+
 def copy_graph(g: Graph, preserve_blank_nodes: bool = True) -> Graph:
     """
     Copy a graph. Creates new blank nodes so that these remain unique to each Graph
@@ -483,3 +491,25 @@ def rewrite_shape_graph(g: Graph) -> Graph:
         # make sure to handle sh:node *after* sh:and
         _inline_sh_node(sg)
     return sg
+
+
+def skip_uri(uri: URIRef) -> bool:
+    """
+    Returns true if the URI should be skipped during processing
+    because it is axiomatic or not expected to be imported.
+
+    Skips URIs in the XSD, SHACL namespaces
+
+    :return: True if the URI should be skipped; false otherwise
+    :rtype: bool
+    """
+    # Now that we have all the templates, we can populate the dependencies.
+    # IGNORES missing XSD imports --- there is really no reason to import the XSD
+    # ontology because the handling is baked into the software processing the RDF
+    # graph. Thus, XSD URIs will always yield import warnings. This is noisy, so we
+    # suppress them.
+    skip_ns = [XSD, SH]
+    for ns in skip_ns:
+        if uri.startswith(ns):
+            return True
+    return False

pyproject.toml

@@ -77,3 +77,6 @@ plugins = "sqlalchemy.ext.mypy.plugin"
 
 [tool.pytest.ini_options]
 log_cli_level = "WARNING"
+markers = [
+    "integration: mark a test as an integration test"
+]

tests/unit/fixtures/inline-dep-test/templates.yml

@@ -0,0 +1,46 @@
+A:
+  body: >
+    @prefix P: <urn:___param___#> .
+    @prefix brick: <https://brickschema.org/schema/Brick#> .
+    P:name a brick:Entity ;
+    brick:hasPoint P:b, P:c, P:d .
+  optional: ['d']
+  dependencies:
+  - template: B
+    args: {"name": "b"}
+  - template: C
+    args: {"name": "c"}
+  - template: D
+    args: {"name": "d"}
+
+B:
+  body: >
+    @prefix P: <urn:___param___#> .
+    @prefix brick: <https://brickschema.org/schema/Brick#> .
+    P:name a brick:B ;
+      brick:hasPart P:bp .
+
+C:
+  body: >
+    @prefix P: <urn:___param___#> .
+    @prefix brick: <https://brickschema.org/schema/Brick#> .
+    P:name a brick:C ;
+      brick:hasPart P:cp .
+
+D:
+  body: >
+    @prefix P: <urn:___param___#> .
+    @prefix brick: <https://brickschema.org/schema/Brick#> .
+    P:name a brick:D ;
+      brick:hasPart P:dp .
+
+A-alt:
+  body: >
+    @prefix P: <urn:___param___#> .
+    @prefix brick: <https://brickschema.org/schema/Brick#> .
+    P:name a brick:Entity ;
+    brick:hasPoint P:b .
+  optional: ['b-bp']
+  dependencies:
+  - template: B
+    args: {"name": "b"}

tests/unit/fixtures/templates/1.yml

@@ -40,5 +40,7 @@ outside-air-damper:
     @prefix bmparam: <urn:___param___#> .
     @prefix brick: <https://brickschema.org/schema/Brick#> .
     bmparam:name a brick:Outside_Air_Damper ;
-          brick:hasPoint bmparam:pos .
+      brick:hasPoint bmparam:pos, bmparam:sen .
     bmparam:pos a brick:Damper_Position_Command .
+    bmparam:sen a brick:Damper_Position_Sensor .
+  optional: ["sen"]

tests/unit/test_template_api.py

@@ -1,3 +1,4 @@
+import pytest
 from rdflib import Graph, Namespace
 
 from buildingmotif import BuildingMOTIF
@@ -17,9 +18,10 @@ def test_template_evaluate(bm: BuildingMOTIF):
     zone = lib.get_template_by_name("zone")
     assert zone.parameters == {"name", "cav"}
 
-    partial = zone.evaluate({"name": BLDG["zone1"]})
-    assert isinstance(partial, Template)
-    assert partial.parameters == {"cav"}
+    with pytest.warns():
+        partial = zone.evaluate({"name": BLDG["zone1"]})
+        assert isinstance(partial, Template)
+        assert partial.parameters == {"cav"}
 
     graph = partial.evaluate({"cav": BLDG["cav1"]})
     assert isinstance(graph, Graph)
@@ -109,9 +111,33 @@ def test_template_inline_dependencies(bm: BuildingMOTIF):
         "sf-ss",
         "sf-st",
         "oad-pos",
+        "oad-sen",
     }
     assert inlined.parameters == preserved_params
 
+    # test optional 'name' param on dependency; this should
+    # preserve optionality of all params in the dependency
+    lib = Library.load(directory="tests/unit/fixtures/inline-dep-test")
+    templ = lib.get_template_by_name("A")
+    assert templ.parameters == {"name", "b", "c", "d"}
+    assert set(templ.optional_args) == {"d"}
+    assert len(templ.get_dependencies()) == 3
+    inlined = templ.inline_dependencies()
+    assert len(inlined.get_dependencies()) == 0
+    assert inlined.parameters == {"name", "b", "c", "b-bp", "c-cp", "d", "d-dp"}
+    assert set(inlined.optional_args) == {"d", "d-dp"}
+
+    # test optional non-'name' parameter on dependency; this should
+    # only make that single parameter optional
+    templ = lib.get_template_by_name("A-alt")
+    assert templ.parameters == {"name", "b"}
+    assert set(templ.optional_args) == {"b-bp"}
+    assert len(templ.get_dependencies()) == 1
+    inlined = templ.inline_dependencies()
+    assert len(inlined.get_dependencies()) == 0
+    assert inlined.parameters == {"name", "b", "b-bp"}
+    assert set(inlined.optional_args) == {"b-bp"}
+
 
 def test_template_evaluate_with_optional(bm: BuildingMOTIF):
     """
@@ -131,6 +157,13 @@ def test_template_evaluate_with_optional(bm: BuildingMOTIF):
     assert isinstance(t, Template)
     assert t.parameters == {"occ"}
 
+    with pytest.warns():
+        t = templ.evaluate(
+            {"name": BLDG["vav"], "zone": BLDG["zone1"]}, require_optional_args=True
+        )
+    assert isinstance(t, Template)
+    assert t.parameters == {"occ"}
+
 
 def test_template_matching(bm: BuildingMOTIF):
     EX = Namespace("urn:ex/")
@@ -151,7 +184,7 @@ def test_template_matching(bm: BuildingMOTIF):
 
     remaining_template = matcher.remaining_template(mapping)
     assert isinstance(remaining_template, Template)
-    assert remaining_template.parameters == {"pos"}
+    assert remaining_template.parameters == {"sen", "pos"}
 
 
 def test_template_matcher_with_graph_target(bm: BuildingMOTIF):