diff --git a/buildingmotif/database/table_connection.py b/buildingmotif/database/table_connection.py index 60a3ba05c..ebb195751 100644 --- a/buildingmotif/database/table_connection.py +++ b/buildingmotif/database/table_connection.py @@ -1,5 +1,6 @@ import logging import uuid +from functools import lru_cache from typing import Dict, List, Tuple from sqlalchemy.engine import Engine @@ -12,7 +13,6 @@ DBTemplate, DepsAssociation, ) -from buildingmotif.utils import get_parameters class TableConnection: @@ -377,10 +377,21 @@ def update_db_template_optional_args( ) db_template.optional_args = optional_args - def add_template_dependency( + def add_template_dependency_preliminary( self, template_id: int, dependency_id: int, args: Dict[str, str] ): - """Create dependency between two templates. + """Creates a *preliminary* dependency between two templates. This dependency + is preliminary because the bindings between the dependent/dependency templates + are *not validated*. This serves to populate the directed acyclic graph of dependencies between templates + before the parameter bindings are checked. This ensures that all of the parameters for + a template *and those in its dependencies* can all be identified and used as part of + the parameter binding check. The upshot of this process is dependencies between two + templates can refer to parameters in a template *or its dependencies*. This is necessary + to support templates that contain many nested components that refer to each other (such + as the s223:mapsTo property). + + **Important:** Be sure to run "check_all_template_dependencies" to ensure all of your templates. + will work as you expect! :param template_id: dependant template id :type template_id: int @@ -390,56 +401,94 @@ def add_template_dependency( :type args: Dict[str, str] :raises ValueError: if all dependee required_params not in args :raises ValueError: if dependant and dependency template don't share a - library """ self.logger.debug( f"Creating depencency from templates with ids: '{template_id}' and: '{dependency_id}'" ) templ = self.get_db_template_by_id(template_id) - graph = self.bm.graph_connection.get_graph(templ.body_id) - params = get_parameters(graph) - dep = self.get_db_template_by_id(dependency_id) + if "name" not in args.keys(): + raise ValueError( + f"The name parameter is required for the dependency '{templ.name}'." + ) + # In the past we had a check here to make sure the two templates were in the same library. + # This has been removed because it wasn't actually necessary, but we may add it back in + # in the future. + relationship = DepsAssociation( + dependant_id=template_id, + dependee_id=dependency_id, + args=args, + ) + + self.bm.session.add(relationship) + self.bm.session.flush() + + def check_all_template_dependencies(self): + """ + Verifies that all template dependencies have valid references to the parameters + in the dependency or (recursively) its dependencies. Raises an exception if any + issues are found. Checks all templates in the database. + """ + # TODO: maybe optimize this to only check recently added templates + # (i.e. maybe those added since last commit?) + for db_templ in self.get_all_db_templates(): + for dep in self.get_db_template_dependencies(db_templ.id): + self.check_template_dependency_relationship(dep) + + @lru_cache(maxsize=128) + def check_template_dependency_relationship(self, dep: DepsAssociation): + """Verify that the dependency between two templates is well-formed. This involves + a series of checks: + - existence of the dependent and dependency templates is performed during the add_ method + - the args keys appear in the dependency, or recursively in a template that is a dependency + of the named dependency + - the args values appear in the dependent template + - there is a 'name' parameter in the dependent template + + :param dep: the dependency object to check + :type dep: DepsAssociation + :raises ValueError: if all dependee required_params not in args + :raises ValueError: if dependant and dependency template don't share a + library + """ + template_id = dep.dependant_id + dependency_id = dep.dependee_id + args = dep.args + self.logger.debug( + f"Creating depencency from templates with ids: '{template_id}' and: '{dependency_id}'" + ) + from buildingmotif.dataclasses import Template + + templ = Template.load(template_id) + params = templ.inline_dependencies().parameters + dep_templ = Template.load(dependency_id) + dep_params = dep_templ.inline_dependencies().parameters # check parameters are valid if not set(args.values()).issubset(params): raise ValueError( - f"In {templ.name} the values of the bindings to {dep.name} must correspond to the " + f"In {templ.name} the values of the bindings to {dep_templ.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"Dependency {dep_templ.name} refers to params {set(args.values()).difference(params)} " + 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) - dep_params = get_parameters(graph) - if not set(args.keys()).issubset(dep_params): + binding_params = set(args.keys()) + if not binding_params.issubset(dep_params): + diff = binding_params.difference(dep_params) raise ValueError( - f"In {templ.name} the keys of the bindings to {dep.name} must correspond to the " - "parameters in the template dependency" + f"In {templ.name} the keys of the bindings to {dep_templ.name} must correspond to the " + "parameters in the template dependency. " + f"The dependency binding uses params {diff} which don't appear in the dependency template. " + f"Available dependency parameters are {dep_params}" ) # TODO: do we need this kind of check? - if "name" not in args.keys(): - raise ValueError( - f"The name parameter is required for the dependency '{templ.name}'" - ) if len(params) > 0 and args["name"] not in params: raise ValueError( "The name parameter of the dependency must be bound to a param in this template." f"'name' was bound to {args['name']} but available params are {params}" ) - # In the past we had a check here to make sure the two templates were in the same library. - # This has been removed because it wasn't actually necessary, but we may add it back in - # in the future. - relationship = DepsAssociation( - dependant_id=template_id, - dependee_id=dependency_id, - args=args, - ) - - self.bm.session.add(relationship) - self.bm.session.flush() - def remove_template_dependency(self, template_id: int, dependency_id: int): """Remove dependency between two templates. diff --git a/buildingmotif/dataclasses/library.py b/buildingmotif/dataclasses/library.py index f25fef29e..b76d11fa7 100644 --- a/buildingmotif/dataclasses/library.py +++ b/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 @@ -252,6 +255,7 @@ def _load_from_ontology( advanced=True, inplace=True, js=True, + allow_warnings=True, ) lib = cls.create(ontology_name, overwrite=overwrite) @@ -378,6 +382,8 @@ def _resolve_template_dependencies( template_id_lookup: Dict[str, int], dependency_cache: Mapping[int, Union[List[_template_dependency], List[dict]]], ): + # two phases here: first, add all of the templates and their dependencies + # to the database but *don't* check that the dependencies are valid yet for template in self.get_templates(): if template.id not in dependency_cache: continue @@ -386,12 +392,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']}" ) @@ -402,6 +404,9 @@ def _resolve_template_dependencies( except Exception as e: logging.warn(f"Warning: could not resolve dependency {dep}") raise e + # check that all dependencies are valid (use parameters that exist, etc) + for template in self.get_templates(): + template.check_dependencies() def _read_yml_file( self, diff --git a/buildingmotif/dataclasses/model.py b/buildingmotif/dataclasses/model.py index 30f1af8ad..22a43ed83 100644 --- a/buildingmotif/dataclasses/model.py +++ b/buildingmotif/dataclasses/model.py @@ -216,6 +216,7 @@ def compile(self, shape_collections: List["ShapeCollection"]): advanced=True, inplace=True, js=True, + allow_warnings=True, ) post_compile_length = len(model_graph) # type: ignore @@ -229,6 +230,7 @@ def compile(self, shape_collections: List["ShapeCollection"]): advanced=True, inplace=True, js=True, + allow_warnings=True, ) post_compile_length = len(model_graph) # type: ignore attempts -= 1 diff --git a/buildingmotif/dataclasses/template.py b/buildingmotif/dataclasses/template.py index f2f92d6db..d1388bc18 100644 --- a/buildingmotif/dataclasses/template.py +++ b/buildingmotif/dataclasses/template.py @@ -1,3 +1,4 @@ +import warnings from collections import Counter from dataclasses import dataclass from itertools import chain @@ -109,7 +110,21 @@ def add_dependency(self, dependency: "Template", args: Dict[str, str]) -> None: :param args: dictionary of dependency arguments :type args: Dict[str, str] """ - self._bm.table_connection.add_template_dependency(self.id, dependency.id, args) + self._bm.table_connection.add_template_dependency_preliminary( + self.id, dependency.id, args + ) + + def check_dependencies(self): + """ + Verifies that all dependencies have valid references to the parameters + in the dependency or (recursively) its dependencies. Raises an exception if any + issues are found. + + It is recommended to call this after *all* templates in a library have been + loaded in to the DB, else this might falsely raise an error. + """ + for dep in self._bm.table_connection.get_db_template_dependencies(self.id): + self._bm.table_connection.check_template_dependency_relationship(dep) def remove_dependency(self, dependency: "Template") -> None: """Remove dependency from template. @@ -122,7 +137,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 +153,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 +165,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 +246,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 +273,31 @@ 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 + templ_optional_args = set(templ.optional_args) + # 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.update(deptempl.parameters) + else: + # otherwise, only add the parameters that are explicitly + # marked as optional *and* appear in this dependency + templ_optional_args.update(deptempl_opt_args) + # ensure that the optional_args includes all params marked as + # optional by the dependency + templ_optional_args.update( + [rename_params[n] for n in deptempl.optional_args] + ) + + # convert our set of optional params to a list and assign to the parent template + templ.optional_args = list(templ_optional_args) + + # append the inlined template into the parent's body templ.body += deptempl.body - templ.optional_args += dep_optional_args return templ @@ -273,6 +306,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 +326,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: 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,21 +363,29 @@ 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]: + def fill( + self, ns: rdflib.Namespace, include_optional: bool = False + ) -> Tuple[Dict[str, Node], rdflib.Graph]: """Evaluates the template with autogenerated bindings within the given namespace. :param ns: namespace to contain the autogenerated entities :type ns: rdflib.Namespace + :param include_optional: if True, invent URIs for optional parameters; ignore if False + :type include_optional: bool :return: a tuple of the bindings used and the resulting graph :rtype: Tuple[Dict[str, Node], rdflib.Graph] """ bindings: Dict[str, Node] = { param: ns[f"{param}_{token_hex(4)}"] for param in self.parameters - if param not in self.optional_args + if include_optional or param not in self.optional_args } res = self.evaluate(bindings) assert isinstance(res, rdflib.Graph) diff --git a/buildingmotif/utils.py b/buildingmotif/utils.py index 735fc0683..f7ee57f60 100644 --- a/buildingmotif/utils.py +++ b/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 without 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 and 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 diff --git a/libraries/ashrae/223p/nrel-templates/connections.yml b/libraries/ashrae/223p/nrel-templates/connections.yml index 800e12535..7d2023ba4 100644 --- a/libraries/ashrae/223p/nrel-templates/connections.yml +++ b/libraries/ashrae/223p/nrel-templates/connections.yml @@ -5,6 +5,8 @@ air-outlet-cp: P:name a s223:OutletConnectionPoint ; s223:mapsTo P:mapsto ; s223:hasMedium s223:Medium-Air . + P:mapsto a s223:OutletConnectionPoint ; + s223:hasMedium s223:Medium-Air . optional: ["mapsto"] water-inlet-cp: @@ -14,6 +16,8 @@ water-inlet-cp: P:name a s223:InletConnectionPoint ; s223:mapsTo P:mapsto ; s223:hasMedium s223:Medium-Water . + P:mapsto a s223:InletConnectionPoint ; + s223:hasMedium s223:Medium-Water . optional: ["mapsto"] water-outlet-cp: @@ -23,6 +27,8 @@ water-outlet-cp: P:name a s223:OutletConnectionPoint ; s223:mapsTo P:mapsto ; s223:hasMedium s223:Medium-Water . + P:mapsto a s223:OutletConnectionPoint ; + s223:hasMedium s223:Medium-Water . optional: ["mapsto"] air-inlet-cp: @@ -32,6 +38,8 @@ air-inlet-cp: P:name a s223:InletConnectionPoint ; s223:mapsTo P:mapsto ; s223:hasMedium s223:Medium-Air . + P:mapsto a s223:InletConnectionPoint ; + s223:hasMedium s223:Medium-Air . optional: ["mapsto"] duct: @@ -55,6 +63,8 @@ zone-air-inlet-cp: P:name a s223:InletZoneConnectionPoint ; s223:mapsTo P:mapsto ; s223:hasMedium s223:Medium-Air . + P:mapsto a s223:InletConnectionPoint ; + s223:hasMedium s223:Medium-Air . optional: ["mapsto"] zone-air-outlet-cp: @@ -64,4 +74,6 @@ zone-air-outlet-cp: P:name a s223:OutletZoneConnectionPoint ; s223:mapsTo P:mapsto ; s223:hasMedium s223:Medium-Air . + P:mapsto a s223:OutletConnectionPoint ; + s223:hasMedium s223:Medium-Air . optional: ["mapsto"] diff --git a/libraries/ashrae/223p/nrel-templates/devices.yml b/libraries/ashrae/223p/nrel-templates/devices.yml index afd7ca915..3b9cc52d2 100644 --- a/libraries/ashrae/223p/nrel-templates/devices.yml +++ b/libraries/ashrae/223p/nrel-templates/devices.yml @@ -310,7 +310,7 @@ heat-exchanger: - template: differential-sensor args: {"name": "B-chw-diff-press-sensor", "property": "B-chw-diff-press", "whereA": "B-in", "whereB": "B-out"} - template: sensor - args: {"name": "chw-flow-sensor", "property": "chw-flow"} + args: {"name": "chw-flow-sensor", "property": "chw-flow", "where": "B-out"} fcu: # TODO: add s223:FCU diff --git a/libraries/ashrae/223p/nrel-templates/systems.yml b/libraries/ashrae/223p/nrel-templates/systems.yml index e83f458ed..04ae9ddcd 100644 --- a/libraries/ashrae/223p/nrel-templates/systems.yml +++ b/libraries/ashrae/223p/nrel-templates/systems.yml @@ -58,9 +58,9 @@ makeup-air-unit: - template: duct args: {"name": "c7", "a": "heating-coil-air-out", "b": "evaporative-cooler-in"} - template: damper - args: {"name": "oad"} + args: {"name": "oad", "in-mapsto": "outside-air"} - template: evaporative-cooler - args: {"name": "evaporative-cooler"} + args: {"name": "evaporative-cooler", "out-mapsto": "air-supply"} - template: filter args: {"name": "pre-filter"} - template: filter @@ -101,9 +101,9 @@ vav-reheat: - template: duct args: {"name": "c0", "a": "rhc-air-out", "b": "dmp-in"} - template: hot-water-coil - args: {"name": "rhc"} + args: {"name": "rhc", "air-in-mapsto": "air-in"} - template: damper - args: {"name": "dmp"} + args: {"name": "dmp", "out-mapsto": "air-out"} - template: air-inlet-cp args: {"name": "air-in"} - template: air-outlet-cp diff --git a/notebooks/223PExample.ipynb b/notebooks/223PExample.ipynb index 42813ccb3..df383e422 100644 --- a/notebooks/223PExample.ipynb +++ b/notebooks/223PExample.ipynb @@ -51,7 +51,9 @@ "cell_type": "code", "execution_count": null, "id": "0c163759-aca8-4abb-9f43-4367f3b51635", - "metadata": {}, + "metadata": { + "tags": [] + }, "outputs": [], "source": [ "# get some templates from the library\n", @@ -161,7 +163,8 @@ "source": [ "# fill in all the extra parameters with invented names\n", "for templ in things:\n", - " _, graph = templ.fill(BLDG)\n", + " print(templ.name)\n", + " _, graph = templ.fill(BLDG, include_optional = True)\n", " bldg.add_graph(graph)" ] }, @@ -169,7 +172,9 @@ "cell_type": "code", "execution_count": null, "id": "870fe01d-cbf3-4b66-b195-d64fca8ac233", - "metadata": {}, + "metadata": { + "tags": [] + }, "outputs": [], "source": [ "# look at finished model\n", @@ -190,12 +195,44 @@ "print(ctx.report_string)" ] }, + { + "cell_type": "code", + "execution_count": null, + "id": "63662fb6-79fb-4902-8e14-898aed6438cc", + "metadata": {}, + "outputs": [], + "source": [ + "bldg.add_graph(mau_templ.fill(BLDG)[1])" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "7e0e4a07-bd74-4941-85a5-26c05ed22285", + "metadata": {}, + "outputs": [], + "source": [ + "ctx = bldg.validate([s223.get_shape_collection()])\n", + "print(ctx.valid)\n", + "print(ctx.report_string)" + ] + }, { "cell_type": "code", "execution_count": null, "id": "d54b3f6c-d5bb-4d21-9482-85db2b5f2797", "metadata": {}, "outputs": [], + "source": [ + "print(mau_templ.fill(BLDG)[1].serialize())\n" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "75cc8512-b81f-4c95-bdef-02d96cd18fe3", + "metadata": {}, + "outputs": [], "source": [] } ], diff --git a/notebooks/Compile-model.ipynb b/notebooks/Compile-model.ipynb index c07c89ee6..9fc27550a 100644 --- a/notebooks/Compile-model.ipynb +++ b/notebooks/Compile-model.ipynb @@ -129,7 +129,13 @@ { "cell_type": "code", "execution_count": 7, - "metadata": {}, + "metadata": { + "collapsed": true, + "jupyter": { + "outputs_hidden": true + }, + "tags": [] + }, "outputs": [ { "name": "stdout", @@ -2943,5 +2949,5 @@ } }, "nbformat": 4, - "nbformat_minor": 2 + "nbformat_minor": 4 } diff --git a/pyproject.toml b/pyproject.toml index 303210931..b45c5f782 100644 --- a/pyproject.toml +++ b/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" +] diff --git a/tests/integration/test_library_validity.py b/tests/integration/test_library_validity.py index 50ece516d..3c620820c 100644 --- a/tests/integration/test_library_validity.py +++ b/tests/integration/test_library_validity.py @@ -28,7 +28,7 @@ def test_223p_library(bm, library_path_223p: Path): print(" ...skipping") continue m = Model.create(MODEL) - _, g = templ.inline_dependencies().fill(MODEL) + _, g = templ.inline_dependencies().fill(MODEL, include_optional=True) assert isinstance(g, Graph), "was not a graph" m.add_graph(g) ctx = m.validate([ont_223p.get_shape_collection()]) diff --git a/tests/unit/database/table_connection/test_db_template.py b/tests/unit/database/table_connection/test_db_template.py index 33ae15ef4..8e15a8ddf 100644 --- a/tests/unit/database/table_connection/test_db_template.py +++ b/tests/unit/database/table_connection/test_db_template.py @@ -208,9 +208,10 @@ def test_add_template_dependency(bm: BuildingMOTIF): dependee_template, ) = create_dependency_test_fixtures(bm) - bm.table_connection.add_template_dependency( + bm.table_connection.add_template_dependency_preliminary( dependant_template.id, dependee_template.id, {"name": "ding", "h2": "dong"} ) + bm.table_connection.check_all_template_dependencies() assert dependant_template.dependencies == [dependee_template] assert dependee_template.dependants == [dependant_template] @@ -230,9 +231,10 @@ def test_add_template_dependency_bad_args(bm: BuildingMOTIF): ) = create_dependency_test_fixtures(bm) with pytest.raises(ValueError): - bm.table_connection.add_template_dependency( + bm.table_connection.add_template_dependency_preliminary( dependant_template.id, dependee_template.id, {"bad": "ding"} ) + bm.table_connection.check_all_template_dependencies() def test_add_template_dependency_already_exist(bm: BuildingMOTIF): @@ -242,14 +244,15 @@ def test_add_template_dependency_already_exist(bm: BuildingMOTIF): dependee_template, ) = create_dependency_test_fixtures(bm) - bm.table_connection.add_template_dependency( + bm.table_connection.add_template_dependency_preliminary( dependant_template.id, dependee_template.id, {"name": "ding", "h2": "dong"} ) with pytest.raises(IntegrityError): - bm.table_connection.add_template_dependency( + bm.table_connection.add_template_dependency_preliminary( dependant_template.id, dependee_template.id, {"name": "ding", "h2": "dong"} ) + bm.table_connection.check_all_template_dependencies() bm.table_connection.bm.session.rollback() @@ -261,9 +264,10 @@ def test_get_dependencies(bm: BuildingMOTIF): dependee_template, ) = create_dependency_test_fixtures(bm) - bm.table_connection.add_template_dependency( + bm.table_connection.add_template_dependency_preliminary( dependant_template.id, dependee_template.id, {"name": "ding", "h2": "dong"} ) + bm.table_connection.check_all_template_dependencies() res = bm.table_connection.get_db_template_dependencies(dependant_template.id) assert len(res) == 1 @@ -280,9 +284,10 @@ def test_remove_dependencies(bm: BuildingMOTIF): dependee_template, ) = create_dependency_test_fixtures(bm) - bm.table_connection.add_template_dependency( + bm.table_connection.add_template_dependency_preliminary( dependant_template.id, dependee_template.id, {"name": "ding", "h2": "dong"} ) + bm.table_connection.check_all_template_dependencies() res = bm.table_connection.get_db_template_dependencies(dependant_template.id) assert len(res) == 1 diff --git a/tests/unit/dataclasses/test_template_dc.py b/tests/unit/dataclasses/test_template_dc.py index 5230e3efd..d357f636d 100644 --- a/tests/unit/dataclasses/test_template_dc.py +++ b/tests/unit/dataclasses/test_template_dc.py @@ -103,6 +103,7 @@ def test_add_dependency(clean_building_motif): assert dependant.get_dependencies() == ( Dependency(dependee.id, {"name": "1", "param": "2"}), ) + dependant.check_dependencies() def test_add_multiple_dependencies(clean_building_motif): @@ -122,6 +123,7 @@ def test_add_multiple_dependencies(clean_building_motif): in dependant.get_dependencies() ) assert len(dependant.get_dependencies()) == 2 + dependant.check_dependencies() def test_add_dependency_bad_args(clean_building_motif): @@ -131,6 +133,7 @@ def test_add_dependency_bad_args(clean_building_motif): with pytest.raises(ValueError): dependant.add_dependency(dependee, {"bad": "xyz"}) + dependant.check_dependencies() def test_add_dependency_already_exist(clean_building_motif): diff --git a/tests/unit/fixtures/inline-dep-test/templates.yml b/tests/unit/fixtures/inline-dep-test/templates.yml new file mode 100644 index 000000000..37efead16 --- /dev/null +++ b/tests/unit/fixtures/inline-dep-test/templates.yml @@ -0,0 +1,119 @@ +A: + body: > + @prefix P: . + @prefix 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: . + @prefix brick: . + P:name a brick:B ; + brick:hasPart P:bp . + +C: + body: > + @prefix P: . + @prefix brick: . + P:name a brick:C ; + brick:hasPart P:cp . + +D: + body: > + @prefix P: . + @prefix brick: . + P:name a brick:D ; + brick:hasPart P:dp . + +A-alt: + body: > + @prefix P: . + @prefix brick: . + P:name a brick:Entity ; + brick:hasPoint P:b . + optional: ['b-bp'] + dependencies: + - template: B + args: {"name": "b"} + +# for test case + +Parent: + body: > + @prefix P: . + @prefix brick: . + P:name a brick:Entity ; + brick:hasPart P:level1 . + dependencies: + - template: Level1 + args: {"name": "level1"} + +Level1: + body: > + @prefix P: . + @prefix brick: . + P:name a brick:Entity ; + brick:hasPart P:level2 . + dependencies: + - template: Level2 + args: {"name": "level2"} + +Level2: + body: > + @prefix P: . + @prefix brick: . + P:name a brick:Entity ; + brick:hasPart P:level3 . + dependencies: + - template: Level3 + args: {"name": "level3"} + +Level3: + body: > + @prefix P: . + @prefix brick: . + P:name a brick:Entity . + + +# for test case +Parent-opt: + body: > + @prefix P: . + @prefix brick: . + P:name a brick:Entity ; + brick:hasPart P:level1 . + optional: ["level1"] + dependencies: + - template: Level1-opt + args: {"name": "level1"} + +Level1-opt: + body: > + @prefix P: . + @prefix brick: . + P:name a brick:Entity ; + brick:hasPart P:level2 . + optional: ["level2"] + dependencies: + - template: Level2-opt + args: {"name": "level2"} + +Level2-opt: + body: > + @prefix P: . + @prefix brick: . + P:name a brick:Entity ; + brick:hasPart P:level3 . + optional: ["level3"] + dependencies: + - template: Level3 + args: {"name": "level3"} diff --git a/tests/unit/fixtures/templates/1.yml b/tests/unit/fixtures/templates/1.yml index e99d8005a..bb6993e31 100644 --- a/tests/unit/fixtures/templates/1.yml +++ b/tests/unit/fixtures/templates/1.yml @@ -40,5 +40,7 @@ outside-air-damper: @prefix bmparam: . @prefix 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"] diff --git a/tests/unit/test_template_api.py b/tests/unit/test_template_api.py index a5c6f7373..80608dd4a 100644 --- a/tests/unit/test_template_api.py +++ b/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,61 @@ 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"} + + # test inlining 2 or more levels + parent = lib.get_template_by_name("Parent") + assert parent.parameters == {"name", "level1"} + inlined = parent.inline_dependencies() + assert inlined.parameters == { + "name", + "level1", + "level1-level2", + "level1-level2-level3", + } + assert len(inlined.optional_args) == 0 + + # test inlining 2 or more levels with optional params + parent = lib.get_template_by_name("Parent-opt") + assert parent.parameters == {"name", "level1"} + inlined = parent.inline_dependencies() + assert inlined.parameters == { + "name", + "level1", + "level1-level2", + "level1-level2-level3", + } + assert set(inlined.optional_args) == { + "level1", + "level1-level2", + "level1-level2-level3", + } + def test_template_evaluate_with_optional(bm: BuildingMOTIF): """ @@ -131,6 +185,22 @@ 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"} + + with pytest.warns(): + partial_templ = templ.evaluate({"name": BLDG["vav"]}) + assert isinstance(partial_templ, Template) + g = partial_templ.evaluate({"zone": BLDG["zone1"]}) + assert isinstance(g, Graph) + t = partial_templ.evaluate({"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 +221,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): diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 64d4ea11f..b9919986b 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -1,15 +1,18 @@ import pyshacl # type: ignore +import pytest from rdflib import Graph, Namespace, URIRef from buildingmotif import BuildingMOTIF from buildingmotif.dataclasses import Model, ShapeCollection -from buildingmotif.namespaces import BRICK, A +from buildingmotif.namespaces import BRICK, SH, XSD, A from buildingmotif.utils import ( PARAM, + _param_name, get_parameters, get_template_parts_from_shape, replace_nodes, rewrite_shape_graph, + skip_uri, ) PREAMBLE = """@prefix bacnet: . @@ -244,7 +247,7 @@ def test_inline_sh_node(bm: BuildingMOTIF): sc.add_graph(new_sg) ctx = model.validate([sc]) - assert not ctx.valid + assert not ctx.valid, ctx.report_string assert ( "Value class is not in classes (brick:Class2, brick:Class3)" in ctx.report_string @@ -254,9 +257,24 @@ def test_inline_sh_node(bm: BuildingMOTIF): in ctx.report_string or "Value class is not in classes (, )" in ctx.report_string - ), ctx.report_string + ) assert ( "Less than 1 values on ->brick:relationship" in ctx.report_string or "Less than 1 values on ->" in ctx.report_string ) + + +def test_param_name(): + good_p = PARAM["abc"] + assert _param_name(good_p) == "abc" + + bad_p = BRICK["abc"] + with pytest.raises(AssertionError): + _param_name(bad_p) + + +def test_skip_uri(): + assert skip_uri(XSD.integer) + assert skip_uri(SH.NodeShape) + assert not skip_uri(BRICK.Sensor)