convert_file_length_units - fix issue converting to imperial units

ifcpatch ConvertLengthUnit - change used units from plural to singular names, to make it consistent across the api

more test coverage - test converting to more units and back from them

src/ifcopenshell-python/ifcopenshell/util/unit.py

@@ -355,12 +355,31 @@ def get_prefix_multiplier(text):
 
 
 def get_unit_name(text: str) -> Union[str, None]:
+    """Get unit name from str, if unit is in SI."""
     text = text.upper().replace("METER", "METRE")
     for name in unit_names:
         if name.replace("_", " ") in text:
             return name
 
 
+def get_unit_name_universal(text: str) -> Union[str, None]:
+    """Get unit name from str, supports both SI and imperial system.
+
+    Can be used to provide units for `convert()`"""
+    text = text.upper().replace("METER", "METRE")
+    for name in unit_names:
+        if name.replace("_", " ") in text:
+            return name
+    for name in imperial_types:
+        if name.upper() in text:
+            return name
+
+
+def get_full_unit_name(unit: ifcopenshell.entity_instance) -> str:
+    prefix = getattr(unit, "Prefix", None) or ""
+    return prefix + unit.Name.upper()
+
+
 def get_si_dimensions(name):
     return si_dimensions.get(name, si_dimensions["OTHERWISE"])
 
@@ -727,17 +746,27 @@ def iter_element_and_attributes_per_type(
             yield element, attr, val
 
 
-def convert_file_length_units(ifc_file: ifcopenshell.file, target_units: str) -> ifcopenshell.file:
+def convert_file_length_units(ifc_file: ifcopenshell.file, target_units: str = "METER") -> ifcopenshell.file:
     """Converts all units in an IFC file to the specified target units. Returns a new file."""
-    prefix = "MILLI" if target_units == "MILLIMETERS" else None
+    prefix = get_prefix(target_units)
+    si_unit = get_unit_name(target_units)
 
     # Copy all elements from the original file to the patched file
     file_patched = ifcopenshell.file.from_string(ifc_file.wrapped_data.to_string())
 
     unit_assignment = get_unit_assignment(file_patched)
 
-    old_length = [u for u in unit_assignment.Units if getattr(u, "UnitType", None) == "LENGTHUNIT"][0]
-    new_length = ifcopenshell.api.run("unit.add_si_unit", file_patched, unit_type="LENGTHUNIT", prefix=prefix)
+    old_length = next(u for u in unit_assignment.Units if getattr(u, "UnitType", None) == "LENGTHUNIT")
+    if si_unit:
+        new_length = ifcopenshell.api.run("unit.add_si_unit", file_patched, unit_type="LENGTHUNIT", prefix=prefix)
+    else:
+        target_units = target_units.lower()
+        if imperial_types.get(target_units) != "LENGTHUNIT":
+            raise Exception(
+                f'Couldn\'t identify target units "{target_units}". '
+                'The method supports singular unit names like "CENTIMETER", "METER", "FOOT", etc.'
+            )
+        new_length = ifcopenshell.api.run("unit.add_conversion_based_unit", file_patched, name=target_units)
 
     # support tuple of tuples, as in IfcCartesianPointList3D.CoordList
     def convert_value(value):

src/ifcopenshell-python/test/util/test_unit_conversion.py

@@ -16,35 +16,52 @@
 # You should have received a copy of the GNU Lesser General Public License
 # along with IfcOpenShell.  If not, see <http://www.gnu.org/licenses/>.
 import pathlib
-
 import pytest
-
 import ifcopenshell.util.unit
+import numpy as np
 
 UNITS_FIXTURE_DIR = pathlib.Path(__file__).parent.parent / "fixtures" / "units"
 
 
 @pytest.mark.parametrize("ifc_file", UNITS_FIXTURE_DIR.glob("*.ifc"))
-def test_file_units_length_convert(ifc_file):
+def test_file_units_length_convert(ifc_file: str):
     f = ifcopenshell.open(ifc_file)
-    project_unit = ifcopenshell.util.unit.get_project_unit(f, "LENGTHUNIT")
-    if project_unit.Prefix == "MILLI":
-        target_units = "METERS"
-        scale = 0.001
-    else:
-        target_units = "MILLIMETERS"
-        scale = 1000
 
-    new_f = ifcopenshell.util.unit.convert_file_length_units(f, target_units)
+    def get_project_unit(f: ifcopenshell.file) -> str:
+        unit = ifcopenshell.util.unit.get_project_unit(f, "LENGTHUNIT")
+        return ifcopenshell.util.unit.get_full_unit_name(unit)
 
-    for element, attr, val in ifcopenshell.util.unit.iter_element_and_attributes_per_type(new_f, "IfcLengthMeasure"):
-        elem_id = element.id()
-        original_element = f.by_id(elem_id)
-        original_val = getattr(original_element, attr.name())
-        def convert_value(value):
-            if not isinstance(value, tuple):
-                return value * scale
-            return tuple(convert_value(v) for v in value)
+    base_project_unit = get_project_unit(f)
+    target_units = ["MILLIMETRE", "METRE", "CENTIMETRE", "INCH", "FOOT"]
 
-        # assert element is equal to original element times scale
-        assert val == convert_value(original_val)
+    def convert_file_and_test(f: ifcopenshell.file, project_unit: str, target_unit: str) -> ifcopenshell.file:
+        scale = ifcopenshell.util.unit.convert(
+            value=1,
+            from_prefix=ifcopenshell.util.unit.get_prefix(project_unit),
+            from_unit=ifcopenshell.util.unit.get_unit_name_universal(project_unit),
+            to_prefix=ifcopenshell.util.unit.get_prefix(target_unit),
+            to_unit=ifcopenshell.util.unit.get_unit_name_universal(target_unit),
+        )
+        new_f = ifcopenshell.util.unit.convert_file_length_units(f, target_unit)
+
+        for element, attr, val in ifcopenshell.util.unit.iter_element_and_attributes_per_type(
+            new_f, "IfcLengthMeasure"
+        ):
+            elem_id = element.id()
+            original_element = f.by_id(elem_id)
+            original_val = getattr(original_element, attr.name())
+
+            def convert_value(value):
+                if not isinstance(value, tuple):
+                    return value * scale
+                return tuple(convert_value(v) for v in value)
+
+            # assert element is equal to original element times scale
+            assert np.allclose([val], [convert_value(original_val)])
+
+        assert get_project_unit(new_f) == target_unit
+        return new_f
+
+    for target_unit in target_units:
+        new_f = convert_file_and_test(f, base_project_unit, target_unit)
+        convert_file_and_test(new_f, target_unit, base_project_unit)

src/ifcpatch/ifcpatch/recipes/ConvertLengthUnit.py

@@ -31,14 +31,14 @@ class Patcher:
         src: str,
         file: ifcopenshell.file,
         logger: Logger,
-        unit: str = "METERS",
+        unit: str = "METER",
     ):
         """Converts the length unit of a model to the specified unit
 
-        Allowed metric units include METERS, MILLIMETERS, CENTIMETERS, etc.
-        Allowed imperial units include INCHES, FEET, MILES.
+        Allowed metric units include METER, MILLIMETER, CENTIMETER, etc.
+        Allowed imperial units include INCH, FOOT, MILE.
 
-        :param unit: The name of the desired unit, defaults to "METERS"
+        :param unit: The name of the desired unit, defaults to "METER"
         :type unit: str
 
         Example:
@@ -46,10 +46,10 @@ class Patcher:
         .. code:: python
 
             # Convert to millimeters
-            ifcpatch.execute({"input": "input.ifc", "file": model, "recipe": "ConvertLengthUnit", "arguments": ["MILLIMETERS"]})
+            ifcpatch.execute({"input": "input.ifc", "file": model, "recipe": "ConvertLengthUnit", "arguments": ["MILLIMETER"]})
 
             # Convert to feet
-            ifcpatch.execute({"input": "input.ifc", "file": model, "recipe": "ConvertLengthUnit", "arguments": ["FEET"]})
+            ifcpatch.execute({"input": "input.ifc", "file": model, "recipe": "ConvertLengthUnit", "arguments": ["FOOT"]})
         """
         self.src = src
         self.file = file

src/ifcpatch/ifcpatch/recipes/MergeProject.py

@@ -79,14 +79,7 @@ class Patcher:
 
     def get_unit_name(self, ifc_file: ifcopenshell.file) -> str:
         length_unit = ifcopenshell.util.unit.get_project_unit(ifc_file, "LENGTHUNIT")
-        names = {
-            "METRE": "METERS",
-            "FOOT": "FEET",
-            "INCH": "INCHES",
-            "MILE": "MILES",
-        }
-        prefix = getattr(length_unit, "Prefix", None) or ""
-        return prefix + names[length_unit.Name.upper()]
+        return ifcopenshell.util.unit.get_full_unit_name(length_unit)
 
     def reuse_existing_contexts(self):
         to_delete = set()

src/ifcpatch/test/test_ConvertLengthUnit.py

@@ -29,10 +29,10 @@ class TestConvertLengthUnit(test.bootstrap.IFC4):
         unit = ifcopenshell.api.run("unit.add_si_unit", self.file, unit_type="LENGTHUNIT", prefix="MILLI")
         ifcopenshell.api.run("unit.assign_unit", self.file, units=[unit])
         output = ifcpatch.execute(
-            {"input": "input.ifc", "file": self.file, "recipe": "ConvertLengthUnit", "arguments": ["METERS"]}
+            {"input": "input.ifc", "file": self.file, "recipe": "ConvertLengthUnit", "arguments": ["METER"]}
         )
         unit = ifcopenshell.util.unit.get_project_unit(output, "LENGTHUNIT")
-        assert unit.Prefix == None
+        assert ifcopenshell.util.unit.get_full_unit_name(unit) == "METRE"
 
 
 class TestConvertLengthUnitIFC2X3(test.bootstrap.IFC2X3, TestConvertLengthUnit):