From 9704bf5848a0d8adb61d7575d98672540471c5ea Mon Sep 17 00:00:00 2001 From: Sarah Date: Tue, 1 Oct 2019 16:43:48 +0100 Subject: [PATCH 01/11] add a test for conversion factor from model s I can look at a shortcut --- tests/cellml_files/simple_model_units.cellml | 14 ++++++++++++++ tests/test_model_units.py | 8 ++++++++ 2 files changed, 22 insertions(+) diff --git a/tests/cellml_files/simple_model_units.cellml b/tests/cellml_files/simple_model_units.cellml index a89f8906..0f986900 100644 --- a/tests/cellml_files/simple_model_units.cellml +++ b/tests/cellml_files/simple_model_units.cellml @@ -10,6 +10,9 @@ + + + @@ -31,4 +34,15 @@ + + + + + + + b_1 + 5 + + + \ No newline at end of file diff --git a/tests/test_model_units.py b/tests/test_model_units.py index e8169d62..6905acd5 100644 --- a/tests/test_model_units.py +++ b/tests/test_model_units.py @@ -58,3 +58,11 @@ def test_units(self, parser_instance, model): assert model.units.summarise_units(equation[1].rhs) == '1 / ms' assert model.units.is_unit_equal(model.units.summarise_units(equation[1].lhs), model.units.summarise_units(equation[1].rhs)) + + def test_conversion_factor(self, parser_instance, model): + model.get_equation_graph(True) # set up the graph - it is not automatic + symbol_b1 = model.get_symbol_by_cmeta_id("b_1") + equation = model.get_equations_for([symbol_b1]) + factor = model.units.get_conversion_factor(1 * model.units.summarise_units(equation[0].lhs), + model.units.ureg('us').units) + assert factor == 1000 From 7c9352ca81a5513f411bd904046ce70cf71c791e Mon Sep 17 00:00:00 2001 From: Sarah Date: Wed, 2 Oct 2019 09:45:22 +0100 Subject: [PATCH 02/11] moved conversion test to more appropriate file --- tests/test_model_units.py | 8 -------- tests/test_unit_conversion.py | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/tests/test_model_units.py b/tests/test_model_units.py index 6905acd5..e8169d62 100644 --- a/tests/test_model_units.py +++ b/tests/test_model_units.py @@ -58,11 +58,3 @@ def test_units(self, parser_instance, model): assert model.units.summarise_units(equation[1].rhs) == '1 / ms' assert model.units.is_unit_equal(model.units.summarise_units(equation[1].lhs), model.units.summarise_units(equation[1].rhs)) - - def test_conversion_factor(self, parser_instance, model): - model.get_equation_graph(True) # set up the graph - it is not automatic - symbol_b1 = model.get_symbol_by_cmeta_id("b_1") - equation = model.get_equations_for([symbol_b1]) - factor = model.units.get_conversion_factor(1 * model.units.summarise_units(equation[0].lhs), - model.units.ureg('us').units) - assert factor == 1000 diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index f58af08a..1c42b9ab 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -10,6 +10,11 @@ def model(): return cellmlmanip.load_model(os.path.join(os.path.dirname(__file__), 'cellml_files', "test_simple_odes.cellml")) +@pytest.fixture +def simple_model(): + return cellmlmanip.load_model(os.path.join(os.path.dirname(__file__), 'cellml_files', "simple_model_units.cellml")) + + def test_add_preferred_custom_unit_name(model): time_var = model.get_symbol_by_ontology_term(OXMETA, "time") assert str(model.units.summarise_units(time_var)) == "ms" @@ -20,3 +25,12 @@ def test_add_preferred_custom_unit_name(model): # again model.units.add_preferred_custom_unit_name('millisecond', [{'prefix': 'milli', 'units': 'second'}]) assert str(model.units.summarise_units(time_var)) == "millisecond" + + +def test_conversion_factor(simple_model): + simple_model.get_equation_graph(True) # set up the graph - it is not automatic + symbol_b1 = simple_model.get_symbol_by_cmeta_id("b_1") + equation = simple_model.get_equations_for([symbol_b1]) + factor = simple_model.units.get_conversion_factor(1 * simple_model.units.summarise_units(equation[0].lhs), + simple_model.units.ureg('us').units) + assert factor == 1000 From 087b33152aab72a14ebfaaea814fa9d0befc087d Mon Sep 17 00:00:00 2001 From: Sarah Date: Wed, 2 Oct 2019 10:30:45 +0100 Subject: [PATCH 03/11] added new conversion function with keywords --- cellmlmanip/units.py | 4 ++++ tests/test_unit_conversion.py | 12 +++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/cellmlmanip/units.py b/cellmlmanip/units.py index e7931e6f..c96d6c65 100644 --- a/cellmlmanip/units.py +++ b/cellmlmanip/units.py @@ -346,6 +346,10 @@ def get_conversion_factor(self, quantity, to_unit): """ return self.convert_to(quantity, to_unit).magnitude + def get_convers_factor(self, to_unit=None, quantity=None, from_unit=None, expression=None): + + return self.convert_to(quantity, to_unit).magnitude + def dimensionally_equivalent(self, symbol1, symbol2): """Returns whether two expressions, symbol1 and symbol2, are dimensionally_equivalent (same units ignorging a calling factor). diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 1c42b9ab..3b5f65de 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -27,10 +27,20 @@ def test_add_preferred_custom_unit_name(model): assert str(model.units.summarise_units(time_var)) == "millisecond" -def test_conversion_factor(simple_model): +def test_conversion_factor_original(simple_model): simple_model.get_equation_graph(True) # set up the graph - it is not automatic symbol_b1 = simple_model.get_symbol_by_cmeta_id("b_1") equation = simple_model.get_equations_for([symbol_b1]) factor = simple_model.units.get_conversion_factor(1 * simple_model.units.summarise_units(equation[0].lhs), simple_model.units.ureg('us').units) assert factor == 1000 + +def test_convers_factor(simple_model): + simple_model.get_equation_graph(True) # set up the graph - it is not automatic + symbol_b1 = simple_model.get_symbol_by_cmeta_id("b_1") + equation = simple_model.get_equations_for([symbol_b1]) + expression = equation[0].lhs + to_unit = simple_model.units.ureg('us').units + from_unit = simple_model.units.summarise_units(expression) + quantity = 1 * from_unit + assert simple_model.units.get_convers_factor(to_unit=to_unit, quantity=quantity) == 1000 From 6a5425154b137780113a08f587fc6b5c10662562 Mon Sep 17 00:00:00 2001 From: Sarah Date: Wed, 2 Oct 2019 12:43:43 +0100 Subject: [PATCH 04/11] function created with new name so I can test without hitting other issues --- cellmlmanip/units.py | 11 +++++++++-- tests/test_unit_conversion.py | 12 ++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/cellmlmanip/units.py b/cellmlmanip/units.py index c96d6c65..500e6227 100644 --- a/cellmlmanip/units.py +++ b/cellmlmanip/units.py @@ -347,8 +347,15 @@ def get_conversion_factor(self, quantity, to_unit): return self.convert_to(quantity, to_unit).magnitude def get_convers_factor(self, to_unit=None, quantity=None, from_unit=None, expression=None): - - return self.convert_to(quantity, to_unit).magnitude + assert to_unit is not None, 'No unit given as target of conversion' + assert quantity is not None or from_unit is not None or expression is not None, \ + 'No unit given as source of conversion' + if from_unit is not None: + return self.convert_to(1 * from_unit, to_unit).magnitude + elif quantity is not None: + return self.convert_to(quantity, to_unit).magnitude + else: + return self.convert_to(1 * self.summarise_units(expression), to_unit).magnitude def dimensionally_equivalent(self, symbol1, symbol2): """Returns whether two expressions, symbol1 and symbol2, diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 3b5f65de..fc373318 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -35,6 +35,7 @@ def test_conversion_factor_original(simple_model): simple_model.units.ureg('us').units) assert factor == 1000 + def test_convers_factor(simple_model): simple_model.get_equation_graph(True) # set up the graph - it is not automatic symbol_b1 = simple_model.get_symbol_by_cmeta_id("b_1") @@ -43,4 +44,15 @@ def test_convers_factor(simple_model): to_unit = simple_model.units.ureg('us').units from_unit = simple_model.units.summarise_units(expression) quantity = 1 * from_unit + # quantity to unit assert simple_model.units.get_convers_factor(to_unit=to_unit, quantity=quantity) == 1000 + # no target unit + with pytest.raises(AssertionError): + simple_model.units.get_convers_factor() + # no source unit + with pytest.raises(AssertionError): + simple_model.units.get_convers_factor(to_unit=to_unit) + # unit to unit + assert simple_model.units.get_convers_factor(to_unit=to_unit, from_unit=from_unit) == 1000 + # expression to unit + assert simple_model.units.get_convers_factor(to_unit=to_unit, expression=expression) == 1000 From edb19b35c0c372fbeb2141c3c1787145ba5d80aa Mon Sep 17 00:00:00 2001 From: Sarah Date: Wed, 2 Oct 2019 13:01:01 +0100 Subject: [PATCH 05/11] reverted to original name and caught uses --- cellmlmanip/units.py | 29 ++++++++++++++++++----------- tests/test_unit_conversion.py | 16 ++++++++-------- tests/test_units.py | 8 ++++---- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/cellmlmanip/units.py b/cellmlmanip/units.py index 500e6227..f572f202 100644 --- a/cellmlmanip/units.py +++ b/cellmlmanip/units.py @@ -338,15 +338,22 @@ def summarise_units(self, expr: sympy.Expr): logger.debug('summarise_units(%s) ⟶ %s', expr, found.units) return found.units - def get_conversion_factor(self, quantity, to_unit): - """Returns the magnitude multiplier required to convert from_unit to to_unit + def get_conversion_factor(self, to_unit=None, from_unit=None, quantity=None, expression=None): + """Returns the magnitude multiplier required to convert a unit to the specified unit. + + Note this will work on either a unit, a quantity or an expression. If more than one + of these arguments is given the result will be calculated on the first encountered + in the order: from_unit, quantity, expression + + :param to_unit: Unit object into which the units should be converted + :param from_unit: the Unit to be converted :param quantity: the Unit to be converted, multiplied by '1' to form a Quantity object - :param to_unit: Unit object into which the first units should be converted - :return the magnitude of the resulting conversion factor - """ - return self.convert_to(quantity, to_unit).magnitude + :param expression: an expression from which the Unit is evaluated before conversion - def get_convers_factor(self, to_unit=None, quantity=None, from_unit=None, expression=None): + :return: the magnitude of the resulting conversion factor + + :throws: AssertionError if no target unit is specified or no source unit is specified + """ assert to_unit is not None, 'No unit given as target of conversion' assert quantity is not None or from_unit is not None or expression is not None, \ 'No unit given as source of conversion' @@ -359,14 +366,14 @@ def get_convers_factor(self, to_unit=None, quantity=None, from_unit=None, expres def dimensionally_equivalent(self, symbol1, symbol2): """Returns whether two expressions, symbol1 and symbol2, - are dimensionally_equivalent (same units ignorging a calling factor). + are dimensionally_equivalent (same units ignoring a calling factor). :param symbol1: the first expression to compare - :param unit2: the second expression to compare + :param symbol2: the second expression to compare :return True if units are equal (regardless of quantity), False otherwise """ try: - self.get_conversion_factor(1 * self.summarise_units(symbol1), - self.summarise_units(symbol2)) + self.get_conversion_factor(from_unit=self.summarise_units(symbol1), + to_unit=self.summarise_units(symbol2)) return True except pint.errors.DimensionalityError: return False diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index fc373318..d52b312e 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -31,12 +31,12 @@ def test_conversion_factor_original(simple_model): simple_model.get_equation_graph(True) # set up the graph - it is not automatic symbol_b1 = simple_model.get_symbol_by_cmeta_id("b_1") equation = simple_model.get_equations_for([symbol_b1]) - factor = simple_model.units.get_conversion_factor(1 * simple_model.units.summarise_units(equation[0].lhs), - simple_model.units.ureg('us').units) + factor = simple_model.units.get_conversion_factor(quantity=1 * simple_model.units.summarise_units(equation[0].lhs), + to_unit=simple_model.units.ureg('us').units) assert factor == 1000 -def test_convers_factor(simple_model): +def test_conversion_factor(simple_model): simple_model.get_equation_graph(True) # set up the graph - it is not automatic symbol_b1 = simple_model.get_symbol_by_cmeta_id("b_1") equation = simple_model.get_equations_for([symbol_b1]) @@ -45,14 +45,14 @@ def test_convers_factor(simple_model): from_unit = simple_model.units.summarise_units(expression) quantity = 1 * from_unit # quantity to unit - assert simple_model.units.get_convers_factor(to_unit=to_unit, quantity=quantity) == 1000 + assert simple_model.units.get_conversion_factor(to_unit=to_unit, quantity=quantity) == 1000 # no target unit with pytest.raises(AssertionError): - simple_model.units.get_convers_factor() + simple_model.units.get_conversion_factor() # no source unit with pytest.raises(AssertionError): - simple_model.units.get_convers_factor(to_unit=to_unit) + simple_model.units.get_conversion_factor(to_unit=to_unit) # unit to unit - assert simple_model.units.get_convers_factor(to_unit=to_unit, from_unit=from_unit) == 1000 + assert simple_model.units.get_conversion_factor(to_unit=to_unit, from_unit=from_unit) == 1000 # expression to unit - assert simple_model.units.get_convers_factor(to_unit=to_unit, expression=expression) == 1000 + assert simple_model.units.get_conversion_factor(to_unit=to_unit, expression=expression) == 1000 diff --git a/tests/test_units.py b/tests/test_units.py index 820a8448..d7dc4c4c 100644 --- a/tests/test_units.py +++ b/tests/test_units.py @@ -94,12 +94,12 @@ def test_quantity_translation(self, quantity_store): def test_conversion_factor(self, quantity_store): ureg = quantity_store.ureg - assert quantity_store.get_conversion_factor(1 * ureg.ms, ureg.second) == 0.001 - assert quantity_store.get_conversion_factor(1 * ureg.volt, ureg.mV) == 1000.0 + assert quantity_store.get_conversion_factor(quantity=1 * ureg.ms, to_unit=ureg.second) == 0.001 + assert quantity_store.get_conversion_factor(quantity=1 * ureg.volt, to_unit=ureg.mV) == 1000.0 assert quantity_store.get_conversion_factor( - 1 * quantity_store.get_quantity('milli_mole'), - quantity_store.get_quantity('mole') + quantity=1 * quantity_store.get_quantity('milli_mole'), + to_unit=quantity_store.get_quantity('mole') ) == 0.001 def test_add_custom_unit(self): From ea4a3af3fa37577840d1425fb450bb1180fa8282 Mon Sep 17 00:00:00 2001 From: Sarah Date: Wed, 2 Oct 2019 14:30:42 +0100 Subject: [PATCH 06/11] add a test for same units --- tests/test_unit_conversion.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index d52b312e..beed05a5 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -56,3 +56,19 @@ def test_conversion_factor(simple_model): assert simple_model.units.get_conversion_factor(to_unit=to_unit, from_unit=from_unit) == 1000 # expression to unit assert simple_model.units.get_conversion_factor(to_unit=to_unit, expression=expression) == 1000 + + +def test_conversion_factor_same_units(simple_model): + simple_model.get_equation_graph(True) # set up the graph - it is not automatic + symbol_b = simple_model.get_symbol_by_cmeta_id("b") + equation = simple_model.get_equations_for([symbol_b]) + expression = equation[1].rhs + to_unit = simple_model.units.ureg('per_ms').units + from_unit = simple_model.units.summarise_units(expression) + quantity = 1 * from_unit + # quantity to unit + assert simple_model.units.get_conversion_factor(to_unit=to_unit, quantity=quantity) == 1 + # unit to unit + assert simple_model.units.get_conversion_factor(to_unit=to_unit, from_unit=from_unit) == 1 + # expression to unit + assert simple_model.units.get_conversion_factor(to_unit=to_unit, expression=expression) == 1 From c358e0084b35402e09076ab6ec0642476e108846 Mon Sep 17 00:00:00 2001 From: Sarah Date: Mon, 14 Oct 2019 10:39:16 +0100 Subject: [PATCH 07/11] add type checking to conversion factor code --- cellmlmanip/units.py | 17 ++++++++-- tests/test_unit_conversion.py | 58 ++++++++++++++++++++++++++++++----- 2 files changed, 65 insertions(+), 10 deletions(-) diff --git a/cellmlmanip/units.py b/cellmlmanip/units.py index f572f202..f930d2cc 100644 --- a/cellmlmanip/units.py +++ b/cellmlmanip/units.py @@ -13,6 +13,7 @@ import sympy from pint.converters import ScaleConverter from pint.definitions import UnitDefinition +from pint.registry import UnitRegistry from sympy.printing.lambdarepr import LambdaPrinter @@ -338,7 +339,11 @@ def summarise_units(self, expr: sympy.Expr): logger.debug('summarise_units(%s) ⟶ %s', expr, found.units) return found.units - def get_conversion_factor(self, to_unit=None, from_unit=None, quantity=None, expression=None): + def get_conversion_factor(self, + to_unit, + from_unit=None, + quantity=None, + expression=None): """Returns the magnitude multiplier required to convert a unit to the specified unit. Note this will work on either a unit, a quantity or an expression. If more than one @@ -354,14 +359,20 @@ def get_conversion_factor(self, to_unit=None, from_unit=None, quantity=None, exp :throws: AssertionError if no target unit is specified or no source unit is specified """ - assert to_unit is not None, 'No unit given as target of conversion' + assert to_unit is not None, 'No unit given as target of conversion; to_unit argument is required' assert quantity is not None or from_unit is not None or expression is not None, \ - 'No unit given as source of conversion' + 'No unit given as source of conversion; please use one of from_unit, quantity or expression' + assert [from_unit, quantity, expression].count(None) == 2, \ + 'Multiple target specified; please use only one of from_unit, quantity or expression' + if from_unit is not None: + assert isinstance(from_unit, self.ureg.Unit), 'from_unit must be of type pint:Unit' return self.convert_to(1 * from_unit, to_unit).magnitude elif quantity is not None: + assert isinstance(quantity, self.ureg.Quantity), 'quantity must be of type pint:Quantity' return self.convert_to(quantity, to_unit).magnitude else: + assert isinstance(expression, sympy.Expr), 'expression must be of type Sympy expression' return self.convert_to(1 * self.summarise_units(expression), to_unit).magnitude def dimensionally_equivalent(self, symbol1, symbol2): diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index beed05a5..842992ec 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -36,7 +36,7 @@ def test_conversion_factor_original(simple_model): assert factor == 1000 -def test_conversion_factor(simple_model): +def test_conversion_factor_bad_types(simple_model): simple_model.get_equation_graph(True) # set up the graph - it is not automatic symbol_b1 = simple_model.get_symbol_by_cmeta_id("b_1") equation = simple_model.get_equations_for([symbol_b1]) @@ -44,16 +44,45 @@ def test_conversion_factor(simple_model): to_unit = simple_model.units.ureg('us').units from_unit = simple_model.units.summarise_units(expression) quantity = 1 * from_unit - # quantity to unit - assert simple_model.units.get_conversion_factor(to_unit=to_unit, quantity=quantity) == 1000 - # no target unit - with pytest.raises(AssertionError): - simple_model.units.get_conversion_factor() # no source unit - with pytest.raises(AssertionError): + try: simple_model.units.get_conversion_factor(to_unit=to_unit) + except AssertionError as err: + assert err.args[0] == 'No unit given as source of conversion; ' \ + 'please use one of from_unit, quantity or expression' + try: + simple_model.units.get_conversion_factor(to_unit) + except AssertionError as err: + assert err.args[0] == 'No unit given as source of conversion; ' \ + 'please use one of from_unit, quantity or expression' + # no target unit + try: + simple_model.units.get_conversion_factor(from_unit=from_unit) + except TypeError as err: + assert err.args[0] == 'get_conversion_factor() missing 1 required positional argument: \'to_unit\'' + # multiple sources + try: + simple_model.units.get_conversion_factor(to_unit, from_unit=from_unit, quantity=quantity) + except AssertionError as err: + assert err.args[0] == 'Multiple target specified; please use only one of from_unit, quantity or expression' + #incorrect types + try: + simple_model.units.get_conversion_factor(to_unit, from_unit=quantity) + except AssertionError as err: + assert err.args[0] == 'from_unit must be of type pint:Unit' + try: + simple_model.units.get_conversion_factor(to_unit, quantity=from_unit) + except AssertionError as err: + assert err.args[0] == 'quantity must be of type pint:Quantity' + try: + simple_model.units.get_conversion_factor(to_unit, expression=quantity) + except AssertionError as err: + assert err.args[0] == 'expression must be of type Sympy expression' + # unit to unit assert simple_model.units.get_conversion_factor(to_unit=to_unit, from_unit=from_unit) == 1000 + # quantity to unit + assert simple_model.units.get_conversion_factor(to_unit=to_unit, quantity=quantity) == 1000 # expression to unit assert simple_model.units.get_conversion_factor(to_unit=to_unit, expression=expression) == 1000 @@ -72,3 +101,18 @@ def test_conversion_factor_same_units(simple_model): assert simple_model.units.get_conversion_factor(to_unit=to_unit, from_unit=from_unit) == 1 # expression to unit assert simple_model.units.get_conversion_factor(to_unit=to_unit, expression=expression) == 1 + +def test_conversion_factor(simple_model): + simple_model.get_equation_graph(True) # set up the graph - it is not automatic + symbol_b = simple_model.get_symbol_by_cmeta_id("b") + equation = simple_model.get_equations_for([symbol_b]) + expression = equation[1].rhs + to_unit = simple_model.units.ureg('per_ms').units + from_unit = simple_model.units.summarise_units(expression) + quantity = 1 * from_unit + # quantity to unit + assert simple_model.units.get_conversion_factor(to_unit=to_unit, quantity=quantity) == 1 + # unit to unit + assert simple_model.units.get_conversion_factor(to_unit=to_unit, from_unit=from_unit) == 1 + # expression to unit + assert simple_model.units.get_conversion_factor(to_unit=to_unit, expression=expression) == 1 From ad4884c4bcbcfdd609189d1ae9f9c842f092cbc4 Mon Sep 17 00:00:00 2001 From: Sarah Date: Tue, 15 Oct 2019 09:50:52 +0100 Subject: [PATCH 08/11] changed docstring to match function --- cellmlmanip/units.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cellmlmanip/units.py b/cellmlmanip/units.py index f930d2cc..319da7aa 100644 --- a/cellmlmanip/units.py +++ b/cellmlmanip/units.py @@ -346,9 +346,8 @@ def get_conversion_factor(self, expression=None): """Returns the magnitude multiplier required to convert a unit to the specified unit. - Note this will work on either a unit, a quantity or an expression. If more than one - of these arguments is given the result will be calculated on the first encountered - in the order: from_unit, quantity, expression + Note this will work on either a unit, a quantity or an expression, but requires only + one of these arguments. :param to_unit: Unit object into which the units should be converted :param from_unit: the Unit to be converted From 0bfcb76aeab9642eea013e1efed1496f9386716d Mon Sep 17 00:00:00 2001 From: Sarah Date: Tue, 15 Oct 2019 15:36:42 +0100 Subject: [PATCH 09/11] make tests more efficient at testing exceptions --- tests/test_unit_conversion.py | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 842992ec..4839a054 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -45,39 +45,24 @@ def test_conversion_factor_bad_types(simple_model): from_unit = simple_model.units.summarise_units(expression) quantity = 1 * from_unit # no source unit - try: + with pytest.raises(AssertionError, match='^No unit given as source*'): simple_model.units.get_conversion_factor(to_unit=to_unit) - except AssertionError as err: - assert err.args[0] == 'No unit given as source of conversion; ' \ - 'please use one of from_unit, quantity or expression' - try: + with pytest.raises(AssertionError, match='^No unit given as source*'): simple_model.units.get_conversion_factor(to_unit) - except AssertionError as err: - assert err.args[0] == 'No unit given as source of conversion; ' \ - 'please use one of from_unit, quantity or expression' + # no target unit - try: + with pytest.raises(TypeError): simple_model.units.get_conversion_factor(from_unit=from_unit) - except TypeError as err: - assert err.args[0] == 'get_conversion_factor() missing 1 required positional argument: \'to_unit\'' # multiple sources - try: + with pytest.raises(AssertionError, match='^Multiple target *'): simple_model.units.get_conversion_factor(to_unit, from_unit=from_unit, quantity=quantity) - except AssertionError as err: - assert err.args[0] == 'Multiple target specified; please use only one of from_unit, quantity or expression' #incorrect types - try: + with pytest.raises(AssertionError, match='^from_unit must be of type pint:Unit'): simple_model.units.get_conversion_factor(to_unit, from_unit=quantity) - except AssertionError as err: - assert err.args[0] == 'from_unit must be of type pint:Unit' - try: + with pytest.raises(AssertionError, match='^quantity must be of type pint:Quantity'): simple_model.units.get_conversion_factor(to_unit, quantity=from_unit) - except AssertionError as err: - assert err.args[0] == 'quantity must be of type pint:Quantity' - try: + with pytest.raises(AssertionError, match='^expression must be of type Sympy expression'): simple_model.units.get_conversion_factor(to_unit, expression=quantity) - except AssertionError as err: - assert err.args[0] == 'expression must be of type Sympy expression' # unit to unit assert simple_model.units.get_conversion_factor(to_unit=to_unit, from_unit=from_unit) == 1000 From 04d6442596328919afa889146ed7a510bd833c2d Mon Sep 17 00:00:00 2001 From: Sarah Date: Tue, 15 Oct 2019 15:39:23 +0100 Subject: [PATCH 10/11] remove repeated test and fix flake8 --- cellmlmanip/units.py | 1 - tests/test_unit_conversion.py | 16 +--------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/cellmlmanip/units.py b/cellmlmanip/units.py index 319da7aa..ea8ef08c 100644 --- a/cellmlmanip/units.py +++ b/cellmlmanip/units.py @@ -13,7 +13,6 @@ import sympy from pint.converters import ScaleConverter from pint.definitions import UnitDefinition -from pint.registry import UnitRegistry from sympy.printing.lambdarepr import LambdaPrinter diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 4839a054..5f8d0670 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -56,7 +56,7 @@ def test_conversion_factor_bad_types(simple_model): # multiple sources with pytest.raises(AssertionError, match='^Multiple target *'): simple_model.units.get_conversion_factor(to_unit, from_unit=from_unit, quantity=quantity) - #incorrect types + # incorrect types with pytest.raises(AssertionError, match='^from_unit must be of type pint:Unit'): simple_model.units.get_conversion_factor(to_unit, from_unit=quantity) with pytest.raises(AssertionError, match='^quantity must be of type pint:Quantity'): @@ -87,17 +87,3 @@ def test_conversion_factor_same_units(simple_model): # expression to unit assert simple_model.units.get_conversion_factor(to_unit=to_unit, expression=expression) == 1 -def test_conversion_factor(simple_model): - simple_model.get_equation_graph(True) # set up the graph - it is not automatic - symbol_b = simple_model.get_symbol_by_cmeta_id("b") - equation = simple_model.get_equations_for([symbol_b]) - expression = equation[1].rhs - to_unit = simple_model.units.ureg('per_ms').units - from_unit = simple_model.units.summarise_units(expression) - quantity = 1 * from_unit - # quantity to unit - assert simple_model.units.get_conversion_factor(to_unit=to_unit, quantity=quantity) == 1 - # unit to unit - assert simple_model.units.get_conversion_factor(to_unit=to_unit, from_unit=from_unit) == 1 - # expression to unit - assert simple_model.units.get_conversion_factor(to_unit=to_unit, expression=expression) == 1 From 95e7c81b1e05d001e453d001a93339b8e8377aed Mon Sep 17 00:00:00 2001 From: Sarah Date: Tue, 15 Oct 2019 16:04:39 +0100 Subject: [PATCH 11/11] make regexpr matching more accurate --- tests/test_unit_conversion.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_unit_conversion.py b/tests/test_unit_conversion.py index 5f8d0670..9e7e2195 100644 --- a/tests/test_unit_conversion.py +++ b/tests/test_unit_conversion.py @@ -45,23 +45,23 @@ def test_conversion_factor_bad_types(simple_model): from_unit = simple_model.units.summarise_units(expression) quantity = 1 * from_unit # no source unit - with pytest.raises(AssertionError, match='^No unit given as source*'): + with pytest.raises(AssertionError, match='^No unit given as source.*'): simple_model.units.get_conversion_factor(to_unit=to_unit) - with pytest.raises(AssertionError, match='^No unit given as source*'): + with pytest.raises(AssertionError, match='^No unit given as source.*'): simple_model.units.get_conversion_factor(to_unit) # no target unit with pytest.raises(TypeError): simple_model.units.get_conversion_factor(from_unit=from_unit) # multiple sources - with pytest.raises(AssertionError, match='^Multiple target *'): + with pytest.raises(AssertionError, match='^Multiple target.*'): simple_model.units.get_conversion_factor(to_unit, from_unit=from_unit, quantity=quantity) # incorrect types - with pytest.raises(AssertionError, match='^from_unit must be of type pint:Unit'): + with pytest.raises(AssertionError, match='^from_unit must be of type pint:Unit$'): simple_model.units.get_conversion_factor(to_unit, from_unit=quantity) - with pytest.raises(AssertionError, match='^quantity must be of type pint:Quantity'): + with pytest.raises(AssertionError, match='^quantity must be of type pint:Quantity$'): simple_model.units.get_conversion_factor(to_unit, quantity=from_unit) - with pytest.raises(AssertionError, match='^expression must be of type Sympy expression'): + with pytest.raises(AssertionError, match='^expression must be of type Sympy expression$'): simple_model.units.get_conversion_factor(to_unit, expression=quantity) # unit to unit