chore: Housekeeping + improved test coverage (#24)

Author: mrkbac

.pre-commit-config.yaml

@@ -4,7 +4,7 @@ default_install_hook_types:
 
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v5.0.0
+    rev: v6.0.0
     hooks:
       - id: check-added-large-files
       - id: check-case-conflict
@@ -23,14 +23,14 @@ repos:
       - id: trailing-whitespace
 
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: "v0.9.10"
+    rev: "v0.13.2"
     hooks:
       - id: ruff
         args: ["--fix"]
       - id: ruff-format
 
   - repo: https://github.com/crate-ci/typos
-    rev: v1.30.1
+    rev: v1.36.3
     hooks:
       - id: typos
 
@@ -40,7 +40,7 @@ repos:
       - id: committed
 
   - repo: https://github.com/python-jsonschema/check-jsonschema
-    rev: 0.31.2
+    rev: 0.34.0
     hooks:
       - id: check-github-workflows
         args: ["--verbose"]

pyproject.toml

@@ -15,22 +15,29 @@ classifiers = [
     "License :: OSI Approved :: GNU General Public License v3 (GPLv3)",
 ]
 
+dynamic = ["version"]
+
 dependencies = [
     "mcap>=1.0.2",
-    "mcap-ros1-support>=0.6.0",
     "mcap-ros2-support>=0.3.0",
     "numpy",
     "pydantic>=2.4.2",
     "tqdm",
-    "pyyaml>=6.0.1",
-    "scipy>=1.11.1",
     "jsonargparse[signatures]>=4.35.0",
     "pointcloud2>=0.2.3",
     "pydantic-yaml>=1.2.1",
     "platformdirs>=4.3.8",
+    "kappe[ros1]",  # remove when pyyaml is no longer imported by jsonargparse
 ]
 
-dynamic = ["version"]
+[project.optional-dependencies]
+all = ["kappe[plugin_image, ros1]"]
+plugin_image = ["pillow>=8.0.0"]
+ros1 = ["mcap-ros1-support>=0.6.0"]
+
+[dependency-groups]
+test = ["pytest-cov>=6.2.1", "pytest>=8.4.1", "kappe[all]", "scipy>=1.11.1"]
+
 
 [project.urls]
 repository = "https://github.com/sensmore/kappe"
@@ -39,13 +46,6 @@ repository = "https://github.com/sensmore/kappe"
 kappe = "kappe.cli:main"
 mcap_to_json = "kappe.utils.mcap_to_json:main"
 json_to_mcap = "kappe.utils.json_to_mcap:main"
-
-[project.optional-dependencies]
-plugin_image = ["pillow>=8.0.0"]
-
-[dependency-groups]
-test = ["pytest-cov>=6.2.1", "pytest>=8.4.1", "kappe[plugin_image]"]
-
 [build-system]
 requires = ["hatchling"]
 build-backend = "hatchling.build"
@@ -82,13 +82,11 @@ ignore = [
     # formater
     'COM812', # missing-trailing-comma
     'ISC001', # single-line-implicit-string-concatenation
-
-    'UP038', # Deprecated rule
 ]
 
 [tool.ruff.lint.per-file-ignores]
 # Relaxed lints for test files
-"test/**/*.py" = ["T201", "SLF001", "ANN201"]
+"test/**/*.py" = ["T201", "SLF001", "ANN201", "PLC0415"]
 
 [tool.ruff.lint.flake8-quotes]
 inline-quotes = 'single'

src/kappe/module/pointcloud.py

@@ -1,8 +1,8 @@
 import numpy as np
 from pointcloud2 import create_cloud, read_points
 from pydantic import BaseModel
-from scipy.spatial.transform import Rotation
 
+from kappe.utils.rotation import rotate_points
 from kappe.utils.settings import SettingEgoBounds, SettingRotation
 from kappe.writer import WrappedDecodedMessage
 
@@ -55,10 +55,9 @@ def point_cloud(cfg: SettingPointCloud, msg: WrappedDecodedMessage) -> None:
 
         quat = cfg.rotation.quaternion
         if quat is not None:
-            rot = Rotation.from_quat(np.array(quat))
             stack = np.column_stack([cloud['x'], cloud['y'], cloud['z']])
 
-            r_cloud = rot.apply(stack)
+            r_cloud = rotate_points(stack, quat)
 
             cloud['x'] = r_cloud[:, 0]
             cloud['y'] = r_cloud[:, 1]

src/kappe/module/tf.py

@@ -1,8 +1,8 @@
 from typing import Any, Literal
 
 from pydantic import BaseModel
-from scipy.spatial.transform import Rotation
 
+from kappe.utils.rotation import quaternion_multiply
 from kappe.utils.settings import SettingRotation, SettingTranslation
 from kappe.writer import WrappedDecodedMessage
 
@@ -119,20 +119,17 @@ def tf_apply_offset(cfg: SettingTF, msg: WrappedDecodedMessage) -> None:
             # Apply rotation offset
             offset_quat = offset_cfg.rotation.quaternion
             # Get current rotation as quaternion (x, y, z, w)
-            current_quat = [
+            current_quat = (
                 transform.transform.rotation.x,
                 transform.transform.rotation.y,
                 transform.transform.rotation.z,
                 transform.transform.rotation.w,
-            ]
+            )
 
-            # Use scipy to multiply quaternions
-            current_rot = Rotation.from_quat(current_quat)
-            offset_rot = Rotation.from_quat(offset_quat)
-            result_rot = current_rot * offset_rot
+            # Multiply quaternions
+            result_quat = quaternion_multiply(current_quat, offset_quat)
 
-            # Get the result quaternion and update the transform
-            result_quat = result_rot.as_quat()
+            # Update the transform
             transform.transform.rotation.x = result_quat[0]
             transform.transform.rotation.y = result_quat[1]
             transform.transform.rotation.z = result_quat[2]
@@ -149,7 +146,8 @@ def tf_remove(cfg: SettingTF, msg: WrappedDecodedMessage) -> bool:
             ros_msg.transforms = [
                 tf for tf in ros_msg.transforms if tf.child_frame_id not in cfg.remove
             ]
-        else:
+        else:  # pragma: no cover
+            # Defensive case already prevented by pydantic
             raise ValueError(f'Invalid value for remove: {cfg.remove}')
         return len(ros_msg.transforms) > 0
     return True

src/kappe/module/timing.py

@@ -3,10 +3,14 @@
 from typing import Any
 
 from mcap.records import Message
-from mcap_ros1._vendor.genpy.rostime import Duration as ROS1Duration
-from mcap_ros1._vendor.genpy.rostime import Time as ROS1Time
 from pydantic import BaseModel
 
+try:
+    from mcap_ros1._vendor.genpy.rostime import Duration as ROS1Duration
+    from mcap_ros1._vendor.genpy.rostime import Time as ROS1Time
+except ImportError:  # pragma: no cover
+    ROS1Duration = None  # type: ignore[assignment]
+    ROS1Time = None  # type: ignore[assignment]
 from kappe.writer import WrappedDecodedMessage
 
 logger = logging.getLogger(__name__)
@@ -93,6 +97,9 @@ def fix_ros1_time(msg: Any) -> None:
     secs -> sec
     nsecs -> nanosec
     """
+    if ROS1Time is None or ROS1Duration is None:  # pragma: no cover
+        raise RuntimeError('ros1 support not available, please install kappe with the "ros1" extra')
+
     if not hasattr(msg, '__slots__'):
         return
 
@@ -102,10 +109,11 @@ def fix_ros1_time(msg: Any) -> None:
         if isinstance(attr, list):
             for i in attr:
                 fix_ros1_time(i)
-
         elif isinstance(attr, ROS1Time | ROS1Duration):
             time = TimeMsg()
             time.sec = attr.secs
             time.nanosec = attr.nsecs
 
             setattr(msg, slot, time)
+        else:
+            fix_ros1_time(attr)

src/kappe/plugin.py

@@ -16,11 +16,11 @@ def __init__(self, **_kwargs: Any) -> None:
 
     @property
     @abstractmethod
-    def output_schema(self) -> str:
+    def output_schema(self) -> str:  # pragma: no cover
         pass
 
     @abstractmethod
-    def convert(self, ros_msg: Any) -> Any:
+    def convert(self, ros_msg: Any) -> Any:  # pragma: no cover
         pass
 
 
@@ -43,12 +43,11 @@ def load_plugin(base_folder: Path | None, plugin_name: str) -> Callable[..., Con
     if '.' in plugin_name:
         pkg_name, class_name = plugin_name.split('.')
 
-    plugin_folders: list[Path | None] = [Path(__file__).parent / 'plugins', base_folder]
+    plugin_folders: list[Path] = [Path(__file__).parent / 'plugins']
+    if base_folder:
+        plugin_folders.append(base_folder)
 
     for path in plugin_folders:
-        if path is None:
-            continue
-
         plugin_file = path / f'{pkg_name}.py'
         if not plugin_file.exists():
             logger.debug('Plugin %s does not exist in %s', plugin_name, path)

src/kappe/plugins/image.py

@@ -1,6 +1,5 @@
 import logging
 from io import BytesIO
-from pathlib import Path
 from typing import Any
 
 from kappe.plugin import ConverterPlugin
@@ -9,7 +8,7 @@
 
 try:
     from PIL import Image
-except ImportError as err:
+except ImportError as err:  # pragma: no cover
     logger.warning('PIL not installed. Install with "pip install pillow"')
     raise ImportError from err
 
@@ -80,15 +79,3 @@ def convert(self, ros_msg: Any) -> Any:
     @property
     def output_schema(self) -> str:
         return 'sensor_msgs/msg/CompressedImage'
-
-
-class SaveCompress(ConverterPlugin):
-    def __init__(self, *, quality: int = 10) -> None:
-        self.quality = quality
-        self.counter = 0
-
-    def convert(self, ros_msg: Any) -> Any:
-        stream = BytesIO(ros_msg.data)
-        img = Image.open(stream)
-        with Path(f'{self.counter:08}.jpeg').open('wb') as f:
-            img.save(f, format='jpeg', quality=self.quality)

src/kappe/utils/json_to_mcap.py

@@ -1,3 +1,4 @@
+import argparse
 import io
 import json
 import logging
@@ -133,8 +134,6 @@ def json_to_mcap(
 
 
 def main() -> None:
-    import argparse
-
     parser = argparse.ArgumentParser(description='Convert JSON to MCAP.')
     parser.add_argument('file', type=Path, help='Path to the input JSONL file.')
     parser.add_argument('-o', '--output', type=Path, required=True, help='Path to the MCAP file.')

src/kappe/utils/mcap_to_json.py

@@ -1,3 +1,4 @@
+import argparse
 import json
 import sys
 from collections.abc import Generator
@@ -112,8 +113,6 @@ def mcap_to_json(
 
 
 def main() -> None:
-    import argparse
-
     parser = argparse.ArgumentParser(description='Convert MCAP file to JSON format.')
     parser.add_argument('file', type=Path, help='Path to the MCAP file.')
     parser.add_argument(

src/kappe/utils/msg_def.py

@@ -1,5 +1,6 @@
 import logging
 from collections import deque
+from functools import lru_cache
 from io import BytesIO
 from pathlib import Path
 from typing import IO
@@ -75,7 +76,7 @@ def _get_cache_dir(distro: ROS2Distro) -> Path:
     )
 
 
-def _rglob_first(folder: list[Path], pattern: str) -> Path | None:
+def _rglob_first(folder: tuple[Path | None, ...], pattern: str) -> Path | None:
     for f in folder:
         if f is None or not f.exists():
             continue
@@ -85,7 +86,7 @@ def _rglob_first(folder: list[Path], pattern: str) -> Path | None:
     return None
 
 
-def _get_msg_def_disk(msg_type: str, folder: list[Path]) -> tuple[str, list[str]] | None:
+def _get_msg_def_disk(msg_type: str, folder: tuple[Path, ...]) -> tuple[str, list[str]] | None:
     pkg_name = msg_type.split('/')[0]
     msg_name = msg_type.split('/')[-1]
 
@@ -121,9 +122,10 @@ def _get_msg_def(
 ) -> tuple[str, list[str]] | None:
     cache_dir = _get_cache_dir(distro)
     _update_cache(cache_dir, distro)
-    return _get_msg_def_disk(msg_type, [folder, cache_dir] if folder else [cache_dir])
+    return _get_msg_def_disk(msg_type, (folder, cache_dir) if folder else (cache_dir,))
 
 
+@lru_cache(maxsize=128)
 def get_message_definition(
     msg_type: str, distro: ROS2Distro, folder: Path | None = None
 ) -> str | None: