From b1083f051f38fa3ffc0b57a4c1ef64624ae86212 Mon Sep 17 00:00:00 2001 From: Filippo Luca Ferretti Date: Fri, 20 Sep 2024 12:12:05 +0200 Subject: [PATCH 1/4] Automatically detect `is_urdf` argument --- src/rod/sdf/sdf.py | 45 +++++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/src/rod/sdf/sdf.py b/src/rod/sdf/sdf.py index e2ba4f0..7a40929 100644 --- a/src/rod/sdf/sdf.py +++ b/src/rod/sdf/sdf.py @@ -54,7 +54,7 @@ def load(sdf: pathlib.Path | str, is_urdf: bool | None = None) -> Sdf: Args: sdf: The SDF resource to load. - is_urdf: Marks the SDF resource as an URDF to convert. + is_urdf: A boolean flag indicating whether the input is a URDF. Returns: The parsed SDF file. @@ -66,29 +66,26 @@ def load(sdf: pathlib.Path | str, is_urdf: bool | None = None) -> Sdf: except ValueError: MAX_PATH = os.pathconf("/", "PC_PATH_MAX") - # Get the SDF/URDF string from the input resource. - # If is_urdf is None and the input resource is a file path, we try to guess - # the is_urdf flag checking the file extension. - - # Check first if it's a Path object - if isinstance(sdf, pathlib.Path): - sdf_string = sdf.read_text() - is_urdf = is_urdf if is_urdf is not None else sdf.suffix == ".urdf" - - # Then, check if it's a string with a path - elif ( - isinstance(sdf, str) - and len(sdf) <= MAX_PATH - and pathlib.Path(sdf).is_file() - ): - sdf_string = pathlib.Path(sdf).read_text(encoding="utf-8") - is_urdf = ( - is_urdf if is_urdf is not None else pathlib.Path(sdf).suffix == ".urdf" - ) - - # Finally, it must be a SDF/URDF string - else: - sdf_string = sdf + # Get the SDF/URDF string from the input resource using match-case + match sdf: + # Case 1: It's a Path object + case pathlib.Path(): + sdf_string = sdf.read_text() + is_urdf = sdf.suffix == ".urdf" + + # Case 2: It's a string with a path + case str() if len(sdf) <= MAX_PATH and pathlib.Path(sdf).is_file(): + sdf_string = pathlib.Path(sdf).read_text(encoding="utf-8") + is_urdf = pathlib.Path(sdf).suffix == ".urdf" + + # Case 3: It's an SDF/URDF string + case str(): + sdf_string = sdf + is_urdf = "" in sdf_string + + # Case 4: Raise an error for unsupported types + case _: + raise TypeError(f"Unsupported type for 'sdf': {type(sdf)}") # Convert SDF to URDF if needed (it requires system executables) if is_urdf: From 17e221f04e3625a0acf1b9a32f8b5ed0c6725017 Mon Sep 17 00:00:00 2001 From: Filippo Luca Ferretti Date: Fri, 20 Sep 2024 12:12:36 +0200 Subject: [PATCH 2/4] Fix error on exception --- src/rod/sdf/sdf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rod/sdf/sdf.py b/src/rod/sdf/sdf.py index 7a40929..2196155 100644 --- a/src/rod/sdf/sdf.py +++ b/src/rod/sdf/sdf.py @@ -98,7 +98,7 @@ def load(sdf: pathlib.Path | str, is_urdf: bool | None = None) -> Sdf: try: xml_dict = xmltodict.parse(xml_input=sdf_string) except Exception as exc: - raise exc("Failed to parse 'sdf' argument") from exc + raise RuntimeError("Failed to parse 'sdf' argument") from exc # Look for the top-level element try: From 804b95c3449bc7cd5097386f346fd4e013fd697b Mon Sep 17 00:00:00 2001 From: Filippo Luca Ferretti Date: Fri, 20 Sep 2024 12:13:02 +0200 Subject: [PATCH 3/4] Remove `is_urdf` argument from tests --- tests/test_urdf_exporter.py | 2 +- tests/test_urdf_parsing.py | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/test_urdf_exporter.py b/tests/test_urdf_exporter.py index 281e779..cfbff49 100644 --- a/tests/test_urdf_exporter.py +++ b/tests/test_urdf_exporter.py @@ -26,7 +26,7 @@ def test_urdf_exporter(robot: Robot) -> None: original_urdf_path = ModelFactory.get_model_description(robot=robot) # Load the URDF (it gets converted to SDF internally). - sdf = rod.Sdf.load(sdf=original_urdf_path, is_urdf=True) + sdf = rod.Sdf.load(sdf=original_urdf_path) # Export the URDF from the in-memory SDF-based description. exported_urdf_string = rod.urdf.exporter.UrdfExporter().to_urdf_string(sdf=sdf) diff --git a/tests/test_urdf_parsing.py b/tests/test_urdf_parsing.py index 8ee3e94..88d592a 100644 --- a/tests/test_urdf_parsing.py +++ b/tests/test_urdf_parsing.py @@ -35,10 +35,6 @@ def test_urdf_parsing(robot: Robot) -> None: with pytest.raises(RuntimeError): _ = rod.Sdf.load(sdf=urdf_path.read_text(), is_urdf=False) - # Check that it fails if is_urdf=False and the resource is an urdf string - with pytest.raises(RuntimeError): - _ = rod.Sdf.load(sdf=urdf_path.read_text(), is_urdf=None) - # The following instead should succeed _ = rod.Sdf.load(sdf=urdf_path, is_urdf=None) _ = rod.Sdf.load(sdf=urdf_path, is_urdf=True) @@ -47,7 +43,7 @@ def test_urdf_parsing(robot: Robot) -> None: _ = rod.Sdf.load(sdf=urdf_path.read_text(), is_urdf=True) # Load once again the urdf - rod_sdf = rod.Sdf.load(sdf=urdf_path, is_urdf=True) + rod_sdf = rod.Sdf.load(sdf=urdf_path) # There should be only one model assert len(rod_sdf.models()) == 1 From 4d27396c3151fc7af39b446e3ea43e37115aa5b9 Mon Sep 17 00:00:00 2001 From: Filippo Luca Ferretti Date: Fri, 20 Sep 2024 12:40:29 +0200 Subject: [PATCH 4/4] Allow user to force `is_urdf` argument --- src/rod/sdf/sdf.py | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/rod/sdf/sdf.py b/src/rod/sdf/sdf.py index 2196155..2abe0aa 100644 --- a/src/rod/sdf/sdf.py +++ b/src/rod/sdf/sdf.py @@ -54,7 +54,7 @@ def load(sdf: pathlib.Path | str, is_urdf: bool | None = None) -> Sdf: Args: sdf: The SDF resource to load. - is_urdf: A boolean flag indicating whether the input is a URDF. + is_urdf: Force the SDF resource to be treated as URDF if the automatic detection fails. Returns: The parsed SDF file. @@ -66,26 +66,28 @@ def load(sdf: pathlib.Path | str, is_urdf: bool | None = None) -> Sdf: except ValueError: MAX_PATH = os.pathconf("/", "PC_PATH_MAX") - # Get the SDF/URDF string from the input resource using match-case - match sdf: - # Case 1: It's a Path object - case pathlib.Path(): - sdf_string = sdf.read_text() - is_urdf = sdf.suffix == ".urdf" - - # Case 2: It's a string with a path - case str() if len(sdf) <= MAX_PATH and pathlib.Path(sdf).is_file(): - sdf_string = pathlib.Path(sdf).read_text(encoding="utf-8") - is_urdf = pathlib.Path(sdf).suffix == ".urdf" - - # Case 3: It's an SDF/URDF string - case str(): - sdf_string = sdf - is_urdf = "" in sdf_string - - # Case 4: Raise an error for unsupported types - case _: - raise TypeError(f"Unsupported type for 'sdf': {type(sdf)}") + if is_urdf is not None: + sdf_string = sdf + else: + match sdf: + # Case 1: It's a Path object + case pathlib.Path(): + sdf_string = sdf.read_text() + is_urdf = sdf.suffix == ".urdf" + + # Case 2: It's a string with a path + case str() if len(sdf) <= MAX_PATH and pathlib.Path(sdf).is_file(): + sdf_string = pathlib.Path(sdf).read_text(encoding="utf-8") + is_urdf = pathlib.Path(sdf).suffix == ".urdf" + + # Case 3: It's an SDF/URDF string + case str(): + sdf_string = sdf + is_urdf = "" in sdf_string + + # Case 4: Raise an error for unsupported types + case _: + raise TypeError(f"Unsupported type for 'sdf': {type(sdf)}") # Convert SDF to URDF if needed (it requires system executables) if is_urdf: