From 7236a716f1f16aea7b3e5806c648eae1bd754872 Mon Sep 17 00:00:00 2001 From: Nathan Moore Date: Sun, 27 Oct 2024 09:09:58 -0600 Subject: [PATCH] Clean up sys-param code that reads parameters (#668) * refactor: remove abandoned sys_param defaults code * call get_param instead of get_defaults * remove another reference to sys-param defaults * another place to handle a lack of defaults from sys-params --- .../geojson_modelica_translator.py | 2 +- .../load_connectors/load_base.py | 2 +- .../load_connectors/teaser.py | 13 ++++-- .../system_parameters/system_parameters.py | 42 ++++--------------- 4 files changed, 18 insertions(+), 41 deletions(-) diff --git a/geojson_modelica_translator/geojson_modelica_translator.py b/geojson_modelica_translator/geojson_modelica_translator.py index 5b3d582b2..e26ff95c6 100644 --- a/geojson_modelica_translator/geojson_modelica_translator.py +++ b/geojson_modelica_translator/geojson_modelica_translator.py @@ -167,7 +167,7 @@ def __init__( self._system_parameters = SystemParameters(sys_params_filepath) - geojson_ids = self._system_parameters.get_default("$.buildings.[*].geojson_id", []) + geojson_ids = self._system_parameters.get_param("$.buildings.[*].geojson_id") self._geojson = UrbanOptGeoJson(geojson_filepath, geojson_ids) # Use different couplings for each district system type diff --git a/geojson_modelica_translator/model_connectors/load_connectors/load_base.py b/geojson_modelica_translator/model_connectors/load_connectors/load_base.py index 900c7adc8..c1d035f85 100644 --- a/geojson_modelica_translator/model_connectors/load_connectors/load_base.py +++ b/geojson_modelica_translator/model_connectors/load_connectors/load_base.py @@ -109,7 +109,7 @@ def add_building(self, urbanopt_building, mapper=None): # TODO: Abstract out the GeoJSON functionality if mapper is None: if self.system_parameters: - for building in self.system_parameters.get_default("$.buildings", []): + for building in self.system_parameters.get_param("$.buildings"): # Only look at buildings in the sys-param file, not necessarily the entire feature file if urbanopt_building.feature.properties["id"] == building["geojson_id"]: try: diff --git a/geojson_modelica_translator/model_connectors/load_connectors/teaser.py b/geojson_modelica_translator/model_connectors/load_connectors/teaser.py index 30e2ecffd..301938f22 100644 --- a/geojson_modelica_translator/model_connectors/load_connectors/teaser.py +++ b/geojson_modelica_translator/model_connectors/load_connectors/teaser.py @@ -266,14 +266,20 @@ def post_process(self, scaffold, keep_original_models=False): ) fraction_latent_person = self.system_parameters.get_param( - "buildings.load_model_parameters.rc.fraction_latent_person", default=1.25 + "buildings.load_model_parameters.rc.fraction_latent_person" ) use_moisture_balance = self.system_parameters.get_param( - "buildings.load_model_parameters.rc.use_moisture_balance", default="false" + "buildings.load_model_parameters.rc.use_moisture_balance" ) + if use_moisture_balance is None: + use_moisture_balance = "false" - n_ports = self.system_parameters.get_param("buildings.load_model_parameters.rc.nPorts", default=0) + # TODO: Determine why we are looking for use_moisture_balance & nPorts in the sys-param file. + # Is this just an allowance for future flexibility? + n_ports = self.system_parameters.get_param("buildings.load_model_parameters.rc.nPorts") + if n_ports is None: + n_ports = 1 # create a new parameter for fraction latent person mofile.add_parameter( @@ -542,7 +548,6 @@ def post_process(self, scaffold, keep_original_models=False): self.building_id, "load_model_parameters.rc.temp_setpoint_cooling" ) ), - # FIXME: pick up default value from schema if not specified in system_parameters, # FYI: Modelica insists on booleans being lowercase, so we need to explicitly set "true" and "false" "has_liquid_heating": "true" if self.system_parameters.get_param_by_id( diff --git a/geojson_modelica_translator/system_parameters/system_parameters.py b/geojson_modelica_translator/system_parameters/system_parameters.py index c1315bd56..c882b7503 100644 --- a/geojson_modelica_translator/system_parameters/system_parameters.py +++ b/geojson_modelica_translator/system_parameters/system_parameters.py @@ -90,62 +90,34 @@ def resolve_paths(self): new_path = Path(filepath) / match.value parse(str(match.full_path)).update(self.param_template, new_path.as_posix()) - # def resolve_defaults(self): - # """This method will expand the default data blocks into all the subsequent custom sections. If the value is - # specificed in the custom block then that will be used, otherwise the default value will be replaced""" - # pass - - def get_default(self, jsonpath, default=None): - """Return either the default in the system parameter file, or the specified default. - - :param jsonpath: string, raw jsonpath to what parameter was being requested - :param default: variant, default value - :return: value - """ - schema_default = self.get_param(jsonpath, impute_default=False) - return schema_default or default - - def get_param(self, jsonpath, data=None, default=None, impute_default=True): - """Return the parameter(s) from a jsonpath. If the default is not specified, then will attempt to read the - default from the "default" section of the file. If there is no default there, then it will use the value - specified as the argument. It is not recommended to use the argument default as those values will not be - configurable. Argument-based defaults should be used sparingly. + def get_param(self, jsonpath, data=None): + """Return the parameter(s) from a jsonpath. :param path: string, period delimited path of the data to retrieve :param data: dict, (optional) the data to parse - :param default: variant, (optional) value to return if can't find the result :return: variant, the value from the data """ if jsonpath is None or jsonpath == "": return None - # If this is the first entry into the method, then set the data to the data = data or self.param_template matches = parse(jsonpath).find(data) - default_value = default - if impute_default: - default_value = self.get_default(jsonpath, default) - results = [] - for index, match in enumerate(matches): - # print(f"Index {index} to update match {match.path} | {match.value} | {match.context}") - if match.value is None: - results.append(default_value) - else: - results.append(match.value) + for match in matches: + results.append(match.value) if len(results) == 1: - # If only one value, then return that value and not a list of values + # If only one value, then return that value and not a list of values. results = results[0] elif len(results) == 0: - return default_value + return None # otherwise return the list of values return results def get_param_by_id(self, param_id, jsonpath): - """Return a parameter for a specific id. This is similar to get_param but allows the user + """Return a parameter for a specific id. This wrapper to get_param allows the user to constrain the data based on the id. :param param_id: string, id of the object to look up in the system parameters file