Merge pull request #290 from specklesystems/gergo/allowUnsupportedUnits

allow string units

src/specklepy/objects/base.py

@@ -18,7 +18,7 @@ from warnings import warn
 
 from stringcase import pascalcase
 
-from specklepy.logging.exceptions import SpeckleException
+from specklepy.logging.exceptions import SpeckleException, SpeckleInvalidUnitException
 from specklepy.objects.units import Units, get_units_from_string
 from specklepy.transports.memory import MemoryTransport
 
@@ -322,7 +322,7 @@ class Base(_RegisteringBase):
     id: Union[str, None] = None
     totalChildrenCount: Union[int, None] = None
     applicationId: Union[str, None] = None
-    _units: Union[Units, None] = None
+    _units: Union[None, str] = None
 
     def __init__(self, **kwargs) -> None:
         super().__init__()
@@ -463,22 +463,19 @@ class Base(_RegisteringBase):
 
     @property
     def units(self) -> Union[str, None]:
-        if self._units:
-            return self._units.value
-        return None
+        return self._units
 
     @units.setter
     def units(self, value: Union[str, Units, None]):
-        if value is None:
-            units = value
+        """While this property accepts any string value, geometry expects units to be specific strings (see Units enum)"""
+        if isinstance(value, str) or value is None:
+            self._units = value
         elif isinstance(value, Units):
-            units: Units = value
+            self._units = value.value
         else:
-            units = get_units_from_string(value)
-        self._units = units
-        # except SpeckleInvalidUnitException as ex:
-        #     warn(f"Units are reset to None. Reason {ex.message}")
-        #     self._units = None
+            raise SpeckleInvalidUnitException(
+                f"Unknown type {type(value)} received for units"
+            )
 
     def get_member_names(self) -> List[str]:
         """Get all of the property names on this object, dynamic or not"""

src/specklepy/objects/units.py

@@ -35,6 +35,7 @@ UNITS_STRINGS = {
     Units.none: ["none", "null"],
 }
 
+
 UNITS_ENCODINGS = {
     Units.none: 0,
     None: 0,
@@ -49,6 +50,20 @@ UNITS_ENCODINGS = {
 }
 
 
+UNIT_SCALE = {
+    Units.none: 1,
+    Units.mm: 0.001,
+    Units.cm: 0.01,
+    Units.m: 1.0,
+    Units.km: 1000.0,
+    Units.inches: 0.0254,
+    Units.feet: 0.3048,
+    Units.yards: 0.9144,
+    Units.miles: 1609.340,
+}
+"""Unit scaling factor to meters"""
+
+
 def get_units_from_string(unit: str) -> Units:
     if not isinstance(unit, str):
         raise SpeckleInvalidUnitException(unit)
@@ -59,10 +74,10 @@ def get_units_from_string(unit: str) -> Units:
     raise SpeckleInvalidUnitException(unit)
 
 
-def get_units_from_encoding(unit: int):
+def get_units_from_encoding(unit: int) -> Units:
     for name, encoding in UNITS_ENCODINGS.items():
         if unit == encoding:
-            return name
+            return name or Units.none
 
     raise SpeckleException(
         message=(
@@ -72,13 +87,36 @@ def get_units_from_encoding(unit: int):
     )
 
 
-def get_encoding_from_units(unit: Union[Units, None]):
+def get_encoding_from_units(unit: Union[Units, str, None]):
+    maybe_sanitized_unit = unit
+    if isinstance(unit, str):
+        for unit_enum, aliases in UNITS_STRINGS.items():
+            if unit in aliases:
+                maybe_sanitized_unit = unit_enum
     try:
-        return UNITS_ENCODINGS[unit]
+        return UNITS_ENCODINGS[maybe_sanitized_unit]
     except KeyError as e:
         raise SpeckleException(
             message=(
-                f"No encoding exists for unit {unit}."
+                f"No encoding exists for unit {maybe_sanitized_unit}."
                 f"Please enter a valid unit to encode (eg {UNITS_ENCODINGS})."
             )
         ) from e
+
+
+def get_scale_factor_from_string(fromUnits: str, toUnits: str) -> float:
+    """Returns a scalar to convert distance values from one unit system to another"""
+    return get_scale_factor(get_units_from_string(fromUnits), get_units_from_string(toUnits))
+
+
+def get_scale_factor(fromUnits: Units, toUnits: Units) -> float:
+    """Returns a scalar to convert distance values from one unit system to another"""
+    return get_scale_factor_to_meters(fromUnits) / get_scale_factor_to_meters(toUnits)
+
+
+def get_scale_factor_to_meters(fromUnits: Units) -> float:
+    """Returns a scalar to convert distance values from one unit system to meters"""
+    if fromUnits not in UNIT_SCALE:
+        raise ValueError(f"Invalid units provided: {fromUnits}")
+
+    return UNIT_SCALE[fromUnits]

tests/intergration/test_serialization.py

@@ -17,7 +17,7 @@ class TestSerialization:
         deserialized = operations.deserialize(serialized)
 
         assert base.get_id() == deserialized.get_id()
-        assert base.units == "mm"
+        assert base.units == "millimetres"
         assert isinstance(base.test_bases[0], Base)
         assert base["@revit_thing"].speckle_type == "SpecialRevitFamily"
         assert base["@detach"].name == deserialized["@detach"].name

tests/unit/test_base.py

@@ -85,14 +85,15 @@ def test_speckle_type_cannot_be_set(base: Base) -> None:
 
 def test_setting_units():
     b = Base(units="foot")
-    assert b.units == "ft"
+    assert b.units == "foot"
 
-    with pytest.raises(SpeckleInvalidUnitException):
-        b.units = "big"
+    # with pytest.raises(SpeckleInvalidUnitException):
+    b.units = "big"
+    assert b.units == "big"
 
     with pytest.raises(SpeckleInvalidUnitException):
         b.units = 7  # invalid args are skipped
-    assert b.units == "ft"
+    assert b.units == "big"
 
     b.units = None  # None should be a valid arg
     assert b.units is None

tests/unit/test_unit_scaling.py

@@ -0,0 +1,56 @@
+
+
+import pytest
+
+from specklepy.objects.units import Units, get_scale_factor
+
+
+@pytest.mark.parametrize(
+    "fromUnits, toUnits, inValue, expectedOutValue",
+    [
+        #To self
+        (Units.km, Units.km, 1.5, 1.5),
+        (Units.km, Units.km, 0, 0),
+        (Units.m, Units.m, 1.5, 1.5),
+        (Units.m, Units.m, 0, 0),
+        (Units.cm, Units.cm, 1.5, 1.5),
+        (Units.cm, Units.cm, 0, 0),
+        (Units.mm, Units.mm, 1.5, 1.5),
+        (Units.mm, Units.mm, 0, 0),
+        (Units.miles, Units.miles, 1.5, 1.5),
+        (Units.miles, Units.miles, 0, 0),
+        (Units.yards, Units.yards, 1.5, 1.5),
+        (Units.yards, Units.yards, 0, 0),
+        (Units.feet, Units.feet, 1.5, 1.5),
+        (Units.feet, Units.feet, 0, 0),
+
+        #To Meters
+        (Units.km, Units.m, 987654.321, 987654321),
+        (Units.m, Units.m, 987654.321, 987654.321),
+        (Units.mm, Units.m, 98765432.1, 98765.4321),
+        (Units.cm, Units.m, 9876543.21, 98765.4321),
+
+        #To negative meters
+        (Units.km, Units.m, -987654.321, -987654321),
+        (Units.m, Units.m,- 987654.321, -987654.321),
+        (Units.mm, Units.m, -98765432.1, -98765.4321),
+        (Units.cm, Units.m, -9876543.21, -98765.4321),
+
+        (Units.m, Units.km, 987654.321, 987.654321),
+        (Units.m, Units.cm, 987654.321, 98765432.1),
+        (Units.m, Units.mm, 987654.321, 987654321),
+
+        #Imperial
+        (Units.miles, Units.m, 123.45, 198673.517),
+        (Units.miles, Units.inches, 123.45, 7821792),
+        (Units.yards, Units.m, 123.45, 112.88268),
+        (Units.yards, Units.inches, 123.45, 4444.2),
+        (Units.feet, Units.m, 123.45, 37.62756),
+        (Units.feet, Units.inches, 123.45, 1481.4),
+        (Units.inches, Units.m, 123.45, 3.13563),
+    ],
+)
+def test_get_scale_factor_between_units(fromUnits: Units, toUnits: Units, inValue: float, expectedOutValue: float):
+    Tolerance = 1e-10
+    actual = inValue * get_scale_factor(fromUnits, toUnits)
+    assert(actual - expectedOutValue < Tolerance)