From 179012bc428119442b03db353a4c82d11202682f Mon Sep 17 00:00:00 2001 From: Logan Drescher Date: Tue, 1 Oct 2024 11:35:21 -0400 Subject: [PATCH 1/7] [WIP] Added alternate logic for ModelAttributeChanges --- biosimulators_utils/sedml/exec.py | 2 + biosimulators_utils/sedml/utils.py | 83 +++++++++++++++++-------- biosimulators_utils/sedml/validation.py | 6 +- 3 files changed, 61 insertions(+), 30 deletions(-) diff --git a/biosimulators_utils/sedml/exec.py b/biosimulators_utils/sedml/exec.py index 0d0d3ded..a0953204 100644 --- a/biosimulators_utils/sedml/exec.py +++ b/biosimulators_utils/sedml/exec.py @@ -166,6 +166,8 @@ def exec_task(task, variables, preprocessed_task=None, log=None, config=None, ** ('\n' + ' ' * 2 * (indent + 2)).join(sorted('`' + output.id + '`' for output in doc.outputs)), )) for i_task, task in enumerate(expected_tasks): + task_status = Status.QUEUED + task_exception = None print('{}Executing task {}: `{}`'.format(' ' * 2 * indent, i_task + 1, task.id)) if config.LOG: diff --git a/biosimulators_utils/sedml/utils.py b/biosimulators_utils/sedml/utils.py index 230f13a2..6d9f1d01 100644 --- a/biosimulators_utils/sedml/utils.py +++ b/biosimulators_utils/sedml/utils.py @@ -5,6 +5,7 @@ :Copyright: 2020, Center for Reproducible Biomedical Modeling :License: MIT """ +import regex from ..log.data_model import Status from ..report.data_model import VariableResults, DataGeneratorResults # noqa: F401 @@ -450,7 +451,14 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non # First pass: Must-be-XML changes: non_xml_changes = [] + possible_changes = (AddElementModelChange, ReplaceElementModelChange, + RemoveElementModelChange, ModelAttributeChange, ComputeModelChange) for change in model.changes: + if not isinstance(change, possible_changes): + error_msg = (f"Change {' ' + change.name if change.name else ''} " + f"of type {change.__class__.__name__} is not supported.") + raise NotImplementedError(error_msg) + if isinstance(change, AddElementModelChange): parents = eval_xpath(model_etree, change.target, change.target_namespaces) @@ -496,30 +504,54 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non parent.remove(element) elif isinstance(change, ModelAttributeChange): - obj_xpath, sep, attr = change.target.rpartition('/@') - if sep != '/@': - change.model = model - non_xml_changes.append(change) - continue - # get object to change - obj_xpath, sep, attr = change.target.rpartition('/@') - if sep != '/@': - raise NotImplementedError('target ' + change.target + ' cannot be changed by XML manipulation, as the target ' - 'is not an attribute of a model element') - objs = eval_xpath(model_etree, obj_xpath, change.target_namespaces) - if validate_unique_xml_targets and len(objs) != 1: - raise ValueError('xpath {} must match a single object'.format(obj_xpath)) - - ns_prefix, _, attr = attr.rpartition(':') - if ns_prefix: - ns = change.target_namespaces.get(ns_prefix, None) - if ns is None: - raise ValueError('No namespace is defined with prefix `{}`'.format(ns_prefix)) - attr = '{{{}}}{}'.format(ns, attr) - - # change value - for obj in objs: - obj.set(attr, change.new_value) + xpath_captures = regex.split("[\[|\]]", change.target) + if len(xpath_captures) != 3 or "@" not in xpath_captures[1] or xpath_captures[2] != "": + # Old method for ModelAttributeChange + obj_xpath, sep, attr = change.target.rpartition('/@') + if sep != '/@': + change.model = model + non_xml_changes.append(change) + continue + # get object to change + obj_xpath, sep, attr = change.target.rpartition('/@') + if sep != '/@': + raise NotImplementedError( + 'target ' + change.target + ' cannot be changed by XML manipulation, as the target ' + 'is not an attribute of a model element') + objs = eval_xpath(model_etree, obj_xpath, change.target_namespaces) + if validate_unique_xml_targets and len(objs) != 1: + raise ValueError('xpath {} must match a single object'.format(obj_xpath)) + + ns_prefix, _, attr = attr.rpartition(':') + if ns_prefix: + ns = change.target_namespaces.get(ns_prefix, None) + if ns is None: + raise ValueError('No namespace is defined with prefix `{}`'.format(ns_prefix)) + attr = '{{{}}}{}'.format(ns, attr) + + # change value + for obj in objs: + obj.set(attr, change.new_value) + else: + # New Method for ModelAttributeChange + xml_target_captures = regex.split("[\@|=]", xpath_captures[1]) + xml_target_captures[2] = xml_target_captures[2][1:-1] + _, target_type, target_value = tuple(xml_target_captures) + xml_model_attribute = eval_xpath(model_etree, change.target, change.target_namespaces) + if validate_unique_xml_targets and len(xml_model_attribute) != 1: + raise ValueError(f'xpath {change.target} must match a single object') + xpath_tiers = [elem for elem in regex.split("/", xpath_captures[0]) if ":" in elem] + if len(xpath_tiers) == 0: + raise ValueError(f'No namespace is defined') + existing_namespace = regex.split(":", xpath_tiers[0])[0] + if change.target_namespaces.get(existing_namespace) is None: + raise ValueError(f'No namespace is defined with prefix `{existing_namespace}`') + # change value + for attribute in xml_model_attribute: + if attribute.get("initialConcentration") is not None: + attribute.set("initialConcentration", change.new_value) + else: + raise ValueError(f"SBML attribute to apply `{change.new_value}` to can not be figured out.") elif isinstance(change, ComputeModelChange): # get the values of model variables referenced by compute model changes @@ -559,9 +591,6 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non for obj in objs: obj.set(attr, new_value) - else: - raise NotImplementedError('Change{} of type {} is not supported.'.format( - ' ' + change.name if change.name else '', change.__class__.__name__)) # Interlude: set up the preprocessed task, if there's a set_value_executor preprocessed_task = None diff --git a/biosimulators_utils/sedml/validation.py b/biosimulators_utils/sedml/validation.py index 03b7d879..fdb4a5ce 100644 --- a/biosimulators_utils/sedml/validation.py +++ b/biosimulators_utils/sedml/validation.py @@ -1179,11 +1179,11 @@ def validate_simulation_type(simulation, types): errors = [] if not isinstance(simulation, types): + valid_types = "\n - ".join(type.__name__ for type in types) errors.append([ - 'Simulation {} of type `{}` is not supported. Simulation must be an instance of one of the following:\n - {}'.format( - simulation.id, simulation.__class__.__name__, '\n - '.join(type.__name__ for type in types)) + f'Simulation {simulation.id} of type `{simulation.__class__.__name__}` is not supported. ' + f'Simulation must be an instance of one of the following:\n - {valid_types}' ]) - return errors From 7c06ffb3d6206eb0fbbb84f38b6f0959ce849945 Mon Sep 17 00:00:00 2001 From: Logan Drescher Date: Wed, 2 Oct 2024 10:31:21 -0400 Subject: [PATCH 2/7] Linting fixes --- biosimulators_utils/sedml/utils.py | 59 ++++++++++++++++++------------ 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/biosimulators_utils/sedml/utils.py b/biosimulators_utils/sedml/utils.py index 6d9f1d01..1d523169 100644 --- a/biosimulators_utils/sedml/utils.py +++ b/biosimulators_utils/sedml/utils.py @@ -14,7 +14,8 @@ from ..xml.utils import eval_xpath from .data_model import (SedBase, SedIdGroupMixin, SedDocument, # noqa: F401 Model, ModelLanguagePattern, ModelChange, ModelAttributeChange, AddElementModelChange, - ReplaceElementModelChange, RemoveElementModelChange, ComputeModelChange, SetValueComputeModelChange, + ReplaceElementModelChange, RemoveElementModelChange, ComputeModelChange, + SetValueComputeModelChange, OneStepSimulation, SteadyStateSimulation, UniformTimeCourseSimulation, Task, RepeatedTask, Output, Report, Plot, Plot2D, Plot3D, DataGenerator, Variable, @@ -320,7 +321,8 @@ def resolve_model_and_apply_xml_changes(orig_model, sed_doc, working_dir, try: model_etree = etree.parse(model.source) except Exception as exception: - raise ValueError('The model could not be parsed because the model is not a valid XML document: {}'.format(str(exception))) + raise ValueError('The model could not be parsed because the model is not a valid XML document: {}'.format( + str(exception))) if model.changes: # Change source here so that tasks point to actual source they can find. @@ -335,7 +337,8 @@ def resolve_model_and_apply_xml_changes(orig_model, sed_doc, working_dir, # write model to file if save_to_file: if temp_model_source is None: - modified_model_file, temp_model_source = tempfile.mkstemp(suffix='.xml', dir=os.path.dirname(model.source)) + modified_model_file, temp_model_source = tempfile.mkstemp(suffix='.xml', + dir=os.path.dirname(model.source)) os.close(modified_model_file) model.source = temp_model_source @@ -452,7 +455,7 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non # First pass: Must-be-XML changes: non_xml_changes = [] possible_changes = (AddElementModelChange, ReplaceElementModelChange, - RemoveElementModelChange, ModelAttributeChange, ComputeModelChange) + RemoveElementModelChange, ModelAttributeChange, ComputeModelChange) for change in model.changes: if not isinstance(change, possible_changes): error_msg = (f"Change {' ' + change.name if change.name else ''} " @@ -481,7 +484,8 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non raise ValueError('xpath {} must match a single object'.format(change.target)) try: - new_elements = etree.parse(io.StringIO('' + change.new_elements + '')).getroot().getchildren() + new_elements = etree.parse( + io.StringIO('' + change.new_elements + '')).getroot().getchildren() except etree.XMLSyntaxError as exception: raise ValueError('`{}` is invalid XML. {}'.format(change.new_elements, str(exception))) @@ -504,7 +508,7 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non parent.remove(element) elif isinstance(change, ModelAttributeChange): - xpath_captures = regex.split("[\[|\]]", change.target) + xpath_captures = regex.split(r"[\[|\]]", change.target) if len(xpath_captures) != 3 or "@" not in xpath_captures[1] or xpath_captures[2] != "": # Old method for ModelAttributeChange obj_xpath, sep, attr = change.target.rpartition('/@') @@ -526,7 +530,7 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non if ns_prefix: ns = change.target_namespaces.get(ns_prefix, None) if ns is None: - raise ValueError('No namespace is defined with prefix `{}`'.format(ns_prefix)) + raise ValueError(f'No namespace is defined with prefix `{ns_prefix}`') attr = '{{{}}}{}'.format(ns, attr) # change value @@ -534,7 +538,7 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non obj.set(attr, change.new_value) else: # New Method for ModelAttributeChange - xml_target_captures = regex.split("[\@|=]", xpath_captures[1]) + xml_target_captures = regex.split(r"[\@|=]", xpath_captures[1]) xml_target_captures[2] = xml_target_captures[2][1:-1] _, target_type, target_value = tuple(xml_target_captures) xml_model_attribute = eval_xpath(model_etree, change.target, change.target_namespaces) @@ -542,7 +546,7 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non raise ValueError(f'xpath {change.target} must match a single object') xpath_tiers = [elem for elem in regex.split("/", xpath_captures[0]) if ":" in elem] if len(xpath_tiers) == 0: - raise ValueError(f'No namespace is defined') + raise ValueError('No namespace is defined') existing_namespace = regex.split(":", xpath_tiers[0])[0] if change.target_namespaces.get(existing_namespace) is None: raise ValueError(f'No namespace is defined with prefix `{existing_namespace}`') @@ -557,12 +561,15 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non # get the values of model variables referenced by compute model changes if variable_values is None: model_etrees = {model.id: model_etree} - iter_variable_values = get_values_of_variable_model_xml_targets_of_model_change(change, sed_doc, model_etrees, working_dir) + iter_variable_values = get_values_of_variable_model_xml_targets_of_model_change(change, sed_doc, + model_etrees, + working_dir) else: iter_variable_values = variable_values # calculate new value - new_value = calc_compute_model_change_new_value(change, variable_values=iter_variable_values, range_values=range_values) + new_value = calc_compute_model_change_new_value(change, variable_values=iter_variable_values, + range_values=range_values) if new_value == int(new_value): new_value = str(int(new_value)) else: @@ -591,7 +598,6 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non for obj in objs: obj.set(attr, new_value) - # Interlude: set up the preprocessed task, if there's a set_value_executor preprocessed_task = None if preprocessed_task_sub_executer: @@ -607,16 +613,18 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non for change in non_xml_changes: if isinstance(change, ModelAttributeChange): if not set_value_executer: - raise NotImplementedError('target ' + change.target + ' cannot be changed by XML manipulation, as the target ' - 'is not an attribute of a model element') + raise NotImplementedError( + 'target ' + change.target + ' cannot be changed by XML manipulation, as the target ' + 'is not an attribute of a model element') else: set_value_executer(change.model, change.target, None, change.new_value, preprocessed_task) elif isinstance(change, ComputeModelChange): obj_xpath, sep, attr = change.target.rpartition('/@') if not set_value_executer: - raise NotImplementedError('target ' + change.target + ' cannot be changed by XML manipulation, as the target ' - 'is not an attribute of a model element') + raise NotImplementedError( + 'target ' + change.target + ' cannot be changed by XML manipulation, as the target ' + 'is not an attribute of a model element') set_value_executer(change.model, change.target, change.symbol, change.new_value, preprocessed_task) return preprocessed_task @@ -771,7 +779,8 @@ def calc_data_generator_results(data_generator, variable_results): else: for aggregate_func in AGGREGATE_MATH_FUNCTIONS: if re.search(aggregate_func + r' *\(', data_generator.math): - msg = 'Evaluation of aggregate mathematical functions such as `{}` is not supported.'.format(aggregate_func) + msg = 'Evaluation of aggregate mathematical functions such as `{}` is not supported.'.format( + aggregate_func) raise NotImplementedError(msg) padded_var_shapes = [] @@ -859,7 +868,8 @@ def calc_data_generators_results(data_generators, variable_results, output, task if vars_failed: status = Status.FAILED - msg = 'Data generator {} cannot be calculated because its variables were not successfully produced.'.format(data_gen.id) + msg = 'Data generator {} cannot be calculated because its variables were not successfully produced.'.format( + data_gen.id) exceptions.append(ValueError(msg)) result = None @@ -1164,7 +1174,8 @@ def resolve_range(range, model_etrees=None): if var.symbol: raise NotImplementedError('Symbols are not supported for variables of functional ranges') if model_etrees[var.model.id] is None: - raise NotImplementedError('Functional ranges that involve variables of non-XML-encoded models are not supported.') + raise NotImplementedError( + 'Functional ranges that involve variables of non-XML-encoded models are not supported.') workspace[var.id] = get_value_of_variable_model_xml_targets(var, model_etrees) # calculate the values of the range @@ -1276,11 +1287,11 @@ def does_model_language_use_xpath_variable_targets(language): :obj:`bool`: :obj:`True`, if the model language is encoded in XML """ return ( - re.match(ModelLanguagePattern.CellML, language) - or re.match(ModelLanguagePattern.CopasiML, language) - or re.match(ModelLanguagePattern.MorpheusML, language) - or re.match(ModelLanguagePattern.SBML, language) - or re.match(ModelLanguagePattern.VCML, language) + re.match(ModelLanguagePattern.CellML, language) + or re.match(ModelLanguagePattern.CopasiML, language) + or re.match(ModelLanguagePattern.MorpheusML, language) + or re.match(ModelLanguagePattern.SBML, language) + or re.match(ModelLanguagePattern.VCML, language) ) From a323d5e3e018b814c8e1e3588dd940779cbae9e4 Mon Sep 17 00:00:00 2001 From: Logan Drescher Date: Fri, 4 Oct 2024 08:47:37 -0400 Subject: [PATCH 3/7] debugger hangs on this statement, so using different iteration method --- biosimulators_utils/sedml/exec.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/biosimulators_utils/sedml/exec.py b/biosimulators_utils/sedml/exec.py index a0953204..02b0507d 100644 --- a/biosimulators_utils/sedml/exec.py +++ b/biosimulators_utils/sedml/exec.py @@ -165,7 +165,8 @@ def exec_task(task, variables, preprocessed_task=None, log=None, config=None, ** ' ' * 2 * (indent + 2), ('\n' + ' ' * 2 * (indent + 2)).join(sorted('`' + output.id + '`' for output in doc.outputs)), )) - for i_task, task in enumerate(expected_tasks): + for i_task in range(0, len(expected_tasks)): + task = expected_tasks[i_task] task_status = Status.QUEUED task_exception = None print('{}Executing task {}: `{}`'.format(' ' * 2 * indent, i_task + 1, task.id)) From 7479d6a8e45a837e59526610a815c9d49390cff2 Mon Sep 17 00:00:00 2001 From: Logan Drescher Date: Fri, 4 Oct 2024 08:49:18 -0400 Subject: [PATCH 4/7] Adding more implicit `ModelAttributeChange` options (with more tests) --- biosimulators_utils/sedml/utils.py | 42 ++++++++++++++++++++---------- tests/sedml/test_sedml_utils.py | 19 ++++++++++++-- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/biosimulators_utils/sedml/utils.py b/biosimulators_utils/sedml/utils.py index 1d523169..6dcc0fc9 100644 --- a/biosimulators_utils/sedml/utils.py +++ b/biosimulators_utils/sedml/utils.py @@ -541,29 +541,38 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non xml_target_captures = regex.split(r"[\@|=]", xpath_captures[1]) xml_target_captures[2] = xml_target_captures[2][1:-1] _, target_type, target_value = tuple(xml_target_captures) - xml_model_attribute = eval_xpath(model_etree, change.target, change.target_namespaces) - if validate_unique_xml_targets and len(xml_model_attribute) != 1: + xml_model_element = eval_xpath(model_etree, change.target, change.target_namespaces) + if validate_unique_xml_targets and len(xml_model_element) != 1: raise ValueError(f'xpath {change.target} must match a single object') xpath_tiers = [elem for elem in regex.split("/", xpath_captures[0]) if ":" in elem] if len(xpath_tiers) == 0: raise ValueError('No namespace is defined') - existing_namespace = regex.split(":", xpath_tiers[0])[0] - if change.target_namespaces.get(existing_namespace) is None: - raise ValueError(f'No namespace is defined with prefix `{existing_namespace}`') + element_type = regex.split(":", xpath_tiers[-1]) + if len(element_type) != 2: + raise ValueError("Unexpected number of tokens in model element xpath") + + namespace_prefix, type_suffix = tuple(element_type) + if change.target_namespaces.get(namespace_prefix) is None: + raise ValueError(f'No namespace is defined with prefix `{namespace_prefix}`') # change value - for attribute in xml_model_attribute: - if attribute.get("initialConcentration") is not None: + for attribute in xml_model_element: + if type_suffix == "species" and attribute.get("initialConcentration") is not None: attribute.set("initialConcentration", change.new_value) + elif type_suffix == "compartment" and attribute.get("size") is not None: + attribute.set("size", change.new_value) + elif type_suffix == "parameter" and attribute.get("value") is not None: + attribute.set("value", change.new_value) else: - raise ValueError(f"SBML attribute to apply `{change.new_value}` to can not be figured out.") + change.model = model + non_xml_changes.append(change) + continue elif isinstance(change, ComputeModelChange): # get the values of model variables referenced by compute model changes if variable_values is None: model_etrees = {model.id: model_etree} - iter_variable_values = get_values_of_variable_model_xml_targets_of_model_change(change, sed_doc, - model_etrees, - working_dir) + iter_variable_values = \ + get_values_of_variable_model_xml_targets_of_model_change(change, sed_doc, model_etrees, working_dir) else: iter_variable_values = variable_values @@ -612,10 +621,15 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non # Second pass: changes that need to be interpreter-based: for change in non_xml_changes: if isinstance(change, ModelAttributeChange): + if not set_value_executer: - raise NotImplementedError( - 'target ' + change.target + ' cannot be changed by XML manipulation, as the target ' - 'is not an attribute of a model element') + xpath_captures = regex.split(r"[\[|\]]", change.target) + if len(xpath_captures) != 3 or "@" not in xpath_captures[1] or xpath_captures[2] != "": + raise NotImplementedError( + 'target ' + change.target + ' cannot be changed by XML manipulation, as the target ' + 'is not an attribute of a model element') + else: + raise ValueError(f"SBML attribute to apply `{change.new_value}` to can not be figured out.") else: set_value_executer(change.model, change.target, None, change.new_value, preprocessed_task) diff --git a/tests/sedml/test_sedml_utils.py b/tests/sedml/test_sedml_utils.py index 10390b36..edbe7993 100644 --- a/tests/sedml/test_sedml_utils.py +++ b/tests/sedml/test_sedml_utils.py @@ -681,11 +681,19 @@ def test_errors(self): utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) change = data_model.ModelAttributeChange( - target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species[@id='Trim']", + target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:spesies[@id='Trim']", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_value='1.9') et = etree.parse(self.FIXTURE_FILENAME) - with self.assertRaises(NotImplementedError): + with self.assertRaises(ValueError): + utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + + change = data_model.ModelAttributeChange( + target="/sbml/model/listOfSpecies/spesies[@id='Trim']", + target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, + new_value='1.9') + et = etree.parse(self.FIXTURE_FILENAME) + with self.assertRaises(ValueError): utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) change = data_model.ModelAttributeChange( @@ -740,6 +748,13 @@ def test_errors(self): with self.assertRaisesRegex(ValueError, 'must match a single object'): utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + change = data_model.ModelAttributeChange( + target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species[@id='Trim']", + target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, + new_value='1.9') + et = etree.parse(self.FIXTURE_FILENAME) + utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + def test_apply_compute_model_change_new_value(self): change = data_model.ComputeModelChange( target="/model/parameter[@id='p1']/@value", From ecb73d460c446a779e8929eb184569ebcbc32192 Mon Sep 17 00:00:00 2001 From: Logan Drescher Date: Fri, 4 Oct 2024 12:29:00 -0400 Subject: [PATCH 5/7] More test fixes and code simplification --- biosimulators_utils/sedml/utils.py | 12 ++------ tests/fixtures/sbml-list-of-species.xml | 4 +++ tests/sedml/test_sedml_utils.py | 41 ++++++++++++++++++++++++- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/biosimulators_utils/sedml/utils.py b/biosimulators_utils/sedml/utils.py index 6dcc0fc9..9c641fe1 100644 --- a/biosimulators_utils/sedml/utils.py +++ b/biosimulators_utils/sedml/utils.py @@ -511,17 +511,13 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non xpath_captures = regex.split(r"[\[|\]]", change.target) if len(xpath_captures) != 3 or "@" not in xpath_captures[1] or xpath_captures[2] != "": # Old method for ModelAttributeChange + # get object to change obj_xpath, sep, attr = change.target.rpartition('/@') if sep != '/@': change.model = model non_xml_changes.append(change) continue - # get object to change - obj_xpath, sep, attr = change.target.rpartition('/@') - if sep != '/@': - raise NotImplementedError( - 'target ' + change.target + ' cannot be changed by XML manipulation, as the target ' - 'is not an attribute of a model element') + objs = eval_xpath(model_etree, obj_xpath, change.target_namespaces) if validate_unique_xml_targets and len(objs) != 1: raise ValueError('xpath {} must match a single object'.format(obj_xpath)) @@ -546,10 +542,8 @@ def apply_changes_to_xml_model(model, model_etree, sed_doc=None, working_dir=Non raise ValueError(f'xpath {change.target} must match a single object') xpath_tiers = [elem for elem in regex.split("/", xpath_captures[0]) if ":" in elem] if len(xpath_tiers) == 0: - raise ValueError('No namespace is defined') - element_type = regex.split(":", xpath_tiers[-1]) - if len(element_type) != 2: raise ValueError("Unexpected number of tokens in model element xpath") + element_type = regex.split(":", xpath_tiers[-1]) namespace_prefix, type_suffix = tuple(element_type) if change.target_namespaces.get(namespace_prefix) is None: diff --git a/tests/fixtures/sbml-list-of-species.xml b/tests/fixtures/sbml-list-of-species.xml index 808e0ac7..ca793408 100644 --- a/tests/fixtures/sbml-list-of-species.xml +++ b/tests/fixtures/sbml-list-of-species.xml @@ -14,5 +14,9 @@ + + + + diff --git a/tests/sedml/test_sedml_utils.py b/tests/sedml/test_sedml_utils.py index edbe7993..fc3e7485 100644 --- a/tests/sedml/test_sedml_utils.py +++ b/tests/sedml/test_sedml_utils.py @@ -688,8 +688,33 @@ def test_errors(self): with self.assertRaises(ValueError): utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + + change = data_model.ModelAttributeChange( + target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:spesies[@id='Trim']/initialConcentration", + target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, + new_value='1.9') + et = etree.parse(self.FIXTURE_FILENAME) + with self.assertRaises(NotImplementedError): + utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + + change = data_model.ModelAttributeChange( + target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:spesies[@id='Trim']", + target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, + new_value='1.9') + et = etree.parse(self.FIXTURE_FILENAME) + with self.assertRaises(ValueError): + utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + + change = data_model.ModelAttributeChange( + target="/:sbml/:model/:listOfSpecies/:species[@id='Trim']", + target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, + new_value='1.9') + et = etree.parse(self.FIXTURE_FILENAME) + with self.assertRaises(etree.XPathEvalError): + utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + change = data_model.ModelAttributeChange( - target="/sbml/model/listOfSpecies/spesies[@id='Trim']", + target="/sbml/model/listOfSpecies/species[@id='Trim']", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_value='1.9') et = etree.parse(self.FIXTURE_FILENAME) @@ -755,6 +780,20 @@ def test_errors(self): et = etree.parse(self.FIXTURE_FILENAME) utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + change = data_model.ModelAttributeChange( + target="/sbml:sbml/sbml:model/sbml:listOfCompartments/sbml:compartment[@id='compartment']", + target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, + new_value='1.9') + et = etree.parse(self.FIXTURE_FILENAME) + utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + + change = data_model.ModelAttributeChange( + target="/sbml:sbml/sbml:model/sbml:listOfParameters/sbml:parameter[@id='parameter_1']", + target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, + new_value='1.9') + et = etree.parse(self.FIXTURE_FILENAME) + utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + def test_apply_compute_model_change_new_value(self): change = data_model.ComputeModelChange( target="/model/parameter[@id='p1']/@value", From 3071eaddac9b3287dcc6182629ed4ce490666fbb Mon Sep 17 00:00:00 2001 From: Logan Drescher Date: Fri, 4 Oct 2024 12:37:50 -0400 Subject: [PATCH 6/7] Get rid of warnings for test --- tests/fixtures/sbml-list-of-species.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fixtures/sbml-list-of-species.xml b/tests/fixtures/sbml-list-of-species.xml index ca793408..03471f8f 100644 --- a/tests/fixtures/sbml-list-of-species.xml +++ b/tests/fixtures/sbml-list-of-species.xml @@ -15,7 +15,7 @@ - + From 0f5be309c09610a95fc61e37ba7bde7219e1a892 Mon Sep 17 00:00:00 2001 From: Logan Drescher Date: Fri, 4 Oct 2024 14:26:04 -0400 Subject: [PATCH 7/7] Attempting to get rid of duplication --- tests/sedml/test_sedml_utils.py | 103 ++++++++++---------------------- 1 file changed, 32 insertions(+), 71 deletions(-) diff --git a/tests/sedml/test_sedml_utils.py b/tests/sedml/test_sedml_utils.py index fc3e7485..c616625d 100644 --- a/tests/sedml/test_sedml_utils.py +++ b/tests/sedml/test_sedml_utils.py @@ -456,34 +456,10 @@ def test_add_multiple_elements_to_single_target_with_different_namespace_prefix( species = et.xpath("/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species", namespaces=get_namespaces_with_prefixes(namespaces)) - num_species = len(species) - species_ids = set([s.get('id') for s in species]) - - # apply changes - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + num_species = len(species) + 2 + species_ids = set([s.get('id') for s in species]) | {'NewSpecies1', 'NewSpecies2'} - # check changes applied - xpath_evaluator = etree.XPathEvaluator(et, namespaces=get_namespaces_with_prefixes(namespaces)) - species = xpath_evaluator("/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species") - self.assertEqual(len(species), num_species + 2) - self.assertEqual(set([s.get('id') for s in species]), species_ids | set(['NewSpecies1', 'NewSpecies2'])) - - # check that changes can be written/read from file - doc = data_model.SedDocument( - models=[ - data_model.Model( - id='model', - language='language', - source='source', - changes=[change], - ), - ] - ) - - filename = os.path.join(self.tmp_dir, 'test.sedml') - io.SedmlSimulationWriter().run(doc, filename) - doc2 = io.SedmlSimulationReader().run(filename) - self.assertTrue(doc2.is_equal(doc)) + self._apply_namespace_changes_and_evaluate_equality(change, et, num_species, namespaces, species_ids) #################### # Incorrect namespace @@ -508,6 +484,10 @@ def test_add_multiple_elements_to_single_target_with_different_namespace_prefix( num_species = len(species) species_ids = set([s.get('id') for s in species]) + self._apply_namespace_changes_and_evaluate_equality(change, et, num_species, namespaces, species_ids) + + def _apply_namespace_changes_and_evaluate_equality(self, change: data_model.ModelChange, et, num_species: int, + namespaces: dict, species_ids: "set[str]"): # apply changes utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) @@ -676,123 +656,104 @@ def test_errors(self): target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species[@id='Trim']/@initialConcentration", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_value='1.9') - et = etree.parse(self.FIXTURE_FILENAME) - with self.assertRaises(NotImplementedError): - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME, NotImplementedError) change = data_model.ModelAttributeChange( target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:spesies[@id='Trim']", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_value='1.9') - et = etree.parse(self.FIXTURE_FILENAME) - with self.assertRaises(ValueError): - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) - + self._attempt_change(change, self.FIXTURE_FILENAME, ValueError) change = data_model.ModelAttributeChange( target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:spesies[@id='Trim']/initialConcentration", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_value='1.9') - et = etree.parse(self.FIXTURE_FILENAME) - with self.assertRaises(NotImplementedError): - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME, NotImplementedError) change = data_model.ModelAttributeChange( target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:spesies[@id='Trim']", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_value='1.9') - et = etree.parse(self.FIXTURE_FILENAME) - with self.assertRaises(ValueError): - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME, ValueError) change = data_model.ModelAttributeChange( target="/:sbml/:model/:listOfSpecies/:species[@id='Trim']", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_value='1.9') - et = etree.parse(self.FIXTURE_FILENAME) - with self.assertRaises(etree.XPathEvalError): - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME, etree.XPathEvalError) change = data_model.ModelAttributeChange( target="/sbml/model/listOfSpecies/species[@id='Trim']", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_value='1.9') - et = etree.parse(self.FIXTURE_FILENAME) - with self.assertRaises(ValueError): - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME, ValueError) change = data_model.ModelAttributeChange( target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species/@initialConcentration", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_value='1.9') - et = etree.parse(self.FIXTURE_FILENAME) - with self.assertRaises(ValueError): - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME, ValueError) namespaces = {'sbml': 'http://www.sbml.org/sbml/level2/version4'} change = data_model.ModelAttributeChange( target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species[@id='Trim']/@sbml2:initialConcentration", target_namespaces=namespaces, new_value='1.9') - et = etree.parse(self.FIXTURE_FILENAME) - with self.assertRaises(ValueError): - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME, ValueError) namespaces['sbml2'] = 'http://www.sbml.org/sbml/level2/version4' - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME) change = data_model.AddElementModelChange( target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species[@id='Trim']", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_elements='<') - et = etree.parse(self.FIXTURE_FILENAME) - with self.assertRaisesRegex(ValueError, 'invalid XML'): - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME, ValueError) change = data_model.AddElementModelChange( target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_elements='1.9') - et = etree.parse(self.FIXTURE_FILENAME) - with self.assertRaisesRegex(ValueError, 'must match a single object'): - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME, ValueError) change = data_model.ReplaceElementModelChange( target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species[@id='Trim']", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_elements='<') - et = etree.parse(self.FIXTURE_FILENAME) - with self.assertRaisesRegex(ValueError, 'invalid XML'): - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME, ValueError) change = data_model.ReplaceElementModelChange( target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_elements='1.9') - et = etree.parse(self.FIXTURE_FILENAME) - with self.assertRaisesRegex(ValueError, 'must match a single object'): - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME, ValueError) change = data_model.ModelAttributeChange( target="/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species[@id='Trim']", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_value='1.9') - et = etree.parse(self.FIXTURE_FILENAME) - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME) change = data_model.ModelAttributeChange( target="/sbml:sbml/sbml:model/sbml:listOfCompartments/sbml:compartment[@id='compartment']", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_value='1.9') - et = etree.parse(self.FIXTURE_FILENAME) - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME) change = data_model.ModelAttributeChange( target="/sbml:sbml/sbml:model/sbml:listOfParameters/sbml:parameter[@id='parameter_1']", target_namespaces={'sbml': 'http://www.sbml.org/sbml/level2/version4'}, new_value='1.9') - et = etree.parse(self.FIXTURE_FILENAME) - utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + self._attempt_change(change, self.FIXTURE_FILENAME) + + def _attempt_change(self, change: data_model.ModelChange, sbml_path: str, + expected_exception: "type[BaseException] | tuple[type[BaseException], ...]" = None): + et = etree.parse(sbml_path) + if expected_exception is not None: + with self.assertRaises(expected_exception): + utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) + else: + utils.apply_changes_to_xml_model(data_model.Model(changes=[change]), et, None, None) def test_apply_compute_model_change_new_value(self): change = data_model.ComputeModelChange(