From 647c3ade209dee380c0e647b65b3d7a44f114e86 Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 2 Aug 2023 14:46:28 +1000 Subject: [PATCH] Pint unit fix (#5381) * Accept conversion of fractional values - e.g. "1/10" is a valid input value - pint dimensions returns strange results sometimes * Add option (default) to return value without units - Handles conversion for "stringish" input values * Update unit tests * Fix return from convert_physical_value method * Update unit tests * Improved checking for conversion code * Call to_base_units first * Conversion depends on whether units are supplied or not * Updates to unit testing * Handle conversion of units for supplier parts - Includes some refactoring --- InvenTree/InvenTree/conversion.py | 65 ++++++++++++++++++++++++++----- InvenTree/InvenTree/tests.py | 63 +++++++++++++++++++++++++++--- InvenTree/company/models.py | 5 ++- InvenTree/part/models.py | 3 +- 4 files changed, 118 insertions(+), 18 deletions(-) diff --git a/InvenTree/InvenTree/conversion.py b/InvenTree/InvenTree/conversion.py index 3427d0403d..68e5721af4 100644 --- a/InvenTree/InvenTree/conversion.py +++ b/InvenTree/InvenTree/conversion.py @@ -70,12 +70,13 @@ def reload_unit_registry(): return reg -def convert_physical_value(value: str, unit: str = None): +def convert_physical_value(value: str, unit: str = None, strip_units=True): """Validate that the provided value is a valid physical quantity. Arguments: value: Value to validate (str) unit: Optional unit to convert to, and validate against + strip_units: If True, strip units from the returned value, and return only the dimension Raises: ValidationError: If the value is invalid or cannot be converted to the specified unit @@ -100,7 +101,7 @@ def convert_physical_value(value: str, unit: str = None): if unit: - if val.units == ureg.dimensionless: + if is_dimensionless(val): # If the provided value is dimensionless, assume that the unit is correct val = ureg.Quantity(value, unit) else: @@ -109,19 +110,65 @@ def convert_physical_value(value: str, unit: str = None): # At this point we *should* have a valid pint value # To double check, look at the maginitude - float(val.magnitude) + float(ureg.Quantity(val.magnitude).magnitude) except (TypeError, ValueError, AttributeError): error = _('Provided value is not a valid number') except (pint.errors.UndefinedUnitError, pint.errors.DefinitionSyntaxError): error = _('Provided value has an invalid unit') - except pint.errors.DimensionalityError: - error = _('Provided value could not be converted to the specified unit') - - if error: if unit: error += f' ({unit})' + except pint.errors.DimensionalityError: + error = _('Provided value could not be converted to the specified unit') + if unit: + error += f' ({unit})' + + except Exception as e: + error = _('Error') + ': ' + str(e) + + if error: raise ValidationError(error) - # Return the converted value - return val + # Calculate the "magnitude" of the value, as a float + # If the value is specified strangely (e.g. as a fraction or a dozen), this can cause isuses + # So, we ensure that it is converted to a floating point value + # If we wish to return a "raw" value, some trickery is required + if unit: + magnitude = ureg.Quantity(val.to(unit)).magnitude + else: + magnitude = ureg.Quantity(val.to_base_units()).magnitude + + magnitude = float(ureg.Quantity(magnitude).to_base_units().magnitude) + + if strip_units: + return magnitude + elif unit or val.units: + return ureg.Quantity(magnitude, unit or val.units) + else: + return ureg.Quantity(magnitude) + + +def is_dimensionless(value): + """Determine if the provided value is 'dimensionless' + + A dimensionless value might look like: + + 0.1 + 1/2 dozen + three thousand + 1.2 dozen + (etc) + """ + ureg = get_unit_registry() + + # Ensure the provided value is in the right format + value = ureg.Quantity(value) + + if value.units == ureg.dimensionless: + return True + + if value.to_base_units().units == ureg.dimensionless: + return True + + # At this point, the value is not dimensionless + return False diff --git a/InvenTree/InvenTree/tests.py b/InvenTree/InvenTree/tests.py index 1bc9ae5680..4e6b50c84c 100644 --- a/InvenTree/InvenTree/tests.py +++ b/InvenTree/InvenTree/tests.py @@ -42,6 +42,26 @@ from .validators import validate_overage class ConversionTest(TestCase): """Tests for conversion of physical units""" + def test_base_units(self): + """Test conversion to specified base units""" + tests = { + "3": 3, + "3 dozen": 36, + "50 dozen kW": 600000, + "1 / 10": 0.1, + "1/2 kW": 500, + "1/2 dozen kW": 6000, + "0.005 MW": 5000, + } + + for val, expected in tests.items(): + q = InvenTree.conversion.convert_physical_value(val, 'W') + + self.assertAlmostEqual(q, expected, 0.01) + + q = InvenTree.conversion.convert_physical_value(val, 'W', strip_units=False) + self.assertAlmostEqual(float(q.magnitude), expected, 0.01) + def test_dimensionless_units(self): """Tests for 'dimensonless' unit quantities""" @@ -54,11 +74,21 @@ class ConversionTest(TestCase): '3 hundred': 300, '2 thousand': 2000, '12 pieces': 12, + '1 / 10': 0.1, + '1/2': 0.5, + '-1 / 16': -0.0625, + '3/2': 1.5, + '1/2 dozen': 6, } for val, expected in tests.items(): - q = InvenTree.conversion.convert_physical_value(val).to_base_units() - self.assertEqual(q.magnitude, expected) + # Convert, and leave units + q = InvenTree.conversion.convert_physical_value(val, strip_units=False) + self.assertAlmostEqual(float(q.magnitude), expected, 0.01) + + # Convert, and strip units + q = InvenTree.conversion.convert_physical_value(val) + self.assertAlmostEqual(q, expected, 0.01) def test_invalid_values(self): """Test conversion of invalid inputs""" @@ -71,11 +101,19 @@ class ConversionTest(TestCase): '--', '+', '++', + '1/0', + '1/-', ] for val in inputs: + # Test with a provided unit with self.assertRaises(ValidationError): - InvenTree.conversion.convert_physical_value(val) + InvenTree.conversion.convert_physical_value(val, 'meter') + + # Test dimensionless + with self.assertRaises(ValidationError): + result = InvenTree.conversion.convert_physical_value(val) + print("Testing invalid value:", val, result) def test_custom_units(self): """Tests for custom unit conversion""" @@ -104,8 +142,23 @@ class ConversionTest(TestCase): reg['hpmm'] # Convert some values - q = InvenTree.conversion.convert_physical_value('1 hpmm', 'henry / km') - self.assertEqual(q.magnitude, 1000000) + tests = { + '1': 1, + '1 hpmm': 1000000, + '1 / 10 hpmm': 100000, + '1 / 100 hpmm': 10000, + '0.3 hpmm': 300000, + '-7hpmm': -7000000, + } + + for val, expected in tests.items(): + # Convert, and leave units + q = InvenTree.conversion.convert_physical_value(val, 'henry / km', strip_units=False) + self.assertAlmostEqual(float(q.magnitude), expected, 0.01) + + # Convert and strip units + q = InvenTree.conversion.convert_physical_value(val, 'henry / km') + self.assertAlmostEqual(q, expected, 0.01) class ValidatorTest(TestCase): diff --git a/InvenTree/company/models.py b/InvenTree/company/models.py index 6334ce6dd9..60bcf05fc5 100644 --- a/InvenTree/company/models.py +++ b/InvenTree/company/models.py @@ -638,11 +638,12 @@ class SupplierPart(MetadataMixin, InvenTreeBarcodeMixin, common.models.MetaMixin try: # Attempt conversion to specified unit native_value = InvenTree.conversion.convert_physical_value( - self.pack_quantity, self.part.units + self.pack_quantity, self.part.units, + strip_units=False ) # If part units are not provided, value must be dimensionless - if not self.part.units and native_value.units not in ['', 'dimensionless']: + if not self.part.units and not InvenTree.conversion.is_dimensionless(native_value): raise ValidationError({ 'pack_quantity': _("Pack units must be compatible with the base part units") }) diff --git a/InvenTree/part/models.py b/InvenTree/part/models.py index 8a17ecefa5..1133189f28 100644 --- a/InvenTree/part/models.py +++ b/InvenTree/part/models.py @@ -3576,8 +3576,7 @@ class PartParameter(MetadataMixin, models.Model): if self.template.units: try: - converted = InvenTree.conversion.convert_physical_value(self.data, self.template.units) - self.data_numeric = float(converted.magnitude) + self.data_numeric = InvenTree.conversion.convert_physical_value(self.data, self.template.units) except (ValidationError, ValueError): self.data_numeric = None