Merge pull request #172 from pharmaverse/custom-libreoffice-path

Author: elong0527

CHANGELOG.md

@@ -1,5 +1,24 @@
 # Changelog
 
+## rtflite 2.4.0
+
+### Breaking changes
+
+- Removed the `executable_path` argument from `RTFDocument.write_docx`; pass
+  `converter=LibreOfficeConverter(executable_path=...)` instead (#172).
+
+### Improvements
+
+- `RTFDocument.write_docx` now accepts a `converter=` instance to configure
+  LibreOffice invocation (including custom executable paths) and to enable
+  reusing a converter across multiple conversions (#172).
+
+### Converters
+
+- `LibreOfficeConverter(executable_path=...)` now accepts `Path` objects
+  and resolves executable names via `PATH` when given
+  (for example, `"soffice"`) (#172).
+
 ## rtflite 2.3.1
 
 ### New features

docs/changelog.md

@@ -1,5 +1,24 @@
 # Changelog
 
+## rtflite 2.4.0
+
+### Breaking changes
+
+- Removed the `executable_path` argument from `RTFDocument.write_docx`; pass
+  `converter=LibreOfficeConverter(executable_path=...)` instead (#172).
+
+### Improvements
+
+- `RTFDocument.write_docx` now accepts a `converter=` instance to configure
+  LibreOffice invocation (including custom executable paths) and to enable
+  reusing a converter across multiple conversions (#172).
+
+### Converters
+
+- `LibreOfficeConverter(executable_path=...)` now accepts `Path` objects
+  and resolves executable names via `PATH` when given
+  (for example, `"soffice"`) (#172).
+
 ## rtflite 2.3.1
 
 ### New features

src/rtflite/convert.py

@@ -1,6 +1,7 @@
 import os
 import platform
 import re
+import shutil
 import subprocess
 from collections.abc import Sequence
 from pathlib import Path
@@ -17,7 +18,7 @@ class LibreOfficeConverter:
     using LibreOffice in headless mode.
 
     Requirements:
-        - LibreOffice 7.3 or later must be installed.
+        - LibreOffice 7.1 or later must be installed.
         - Automatically finds LibreOffice in standard installation paths.
         - For custom installations, provide `executable_path` parameter.
 
@@ -26,39 +27,72 @@ class LibreOfficeConverter:
         This makes it suitable for server environments and automated workflows.
     """
 
-    def __init__(self, executable_path: str | None = None):
+    def __init__(self, executable_path: str | Path | None = None):
         """Initialize converter with optional executable path.
 
         Args:
-            executable_path: Path to LibreOffice executable. If None, searches
-                standard installation locations for each platform.
+            executable_path: Path (or executable name) to LibreOffice. If None,
+                searches standard installation locations for each platform.
 
         Raises:
             FileNotFoundError: If LibreOffice executable cannot be found.
             ValueError: If LibreOffice version is below minimum requirement.
         """
-        self.executable_path = executable_path or self._find_executable()
-        if not self.executable_path:
-            raise FileNotFoundError("Can't find LibreOffice executable.")
+        self.executable_path = self._resolve_executable_path(executable_path)
 
         self._verify_version()
 
-    def _find_executable(self) -> str | None:
+    def _resolve_executable_path(self, executable_path: str | Path | None) -> Path:
+        """Resolve the LibreOffice executable path."""
+        if executable_path is None:
+            found_executable = self._find_executable()
+            if found_executable is None:
+                raise FileNotFoundError("Can't find LibreOffice executable.")
+            return found_executable
+
+        executable = os.fspath(executable_path)
+        expanded = os.path.expanduser(executable)
+        candidate = Path(expanded)
+        candidate_str = str(candidate)
+        looks_like_path = (
+            candidate.is_absolute()
+            or os.sep in candidate_str
+            or (os.altsep is not None and os.altsep in candidate_str)
+        )
+        if looks_like_path:
+            if candidate.is_file():
+                return candidate
+            raise FileNotFoundError(
+                f"LibreOffice executable not found at: {candidate}."
+            )
+
+        resolved_executable = shutil.which(executable)
+        if resolved_executable is None:
+            raise FileNotFoundError(f"Can't find LibreOffice executable: {executable}.")
+        return Path(resolved_executable)
+
+    def _find_executable(self) -> Path | None:
         """Find LibreOffice executable in default locations."""
+        for name in ("soffice", "libreoffice"):
+            resolved = shutil.which(name)
+            if resolved is not None:
+                return Path(resolved)
+
         system = platform.system()
         if system not in DEFAULT_PATHS:
             raise RuntimeError(f"Unsupported operating system: {system}.")
 
         for path in DEFAULT_PATHS[system]:
-            if os.path.isfile(path):
-                return path
+            candidate = Path(path)
+            if candidate.is_file():
+                return candidate
         return None
 
     def _verify_version(self):
         """Verify LibreOffice version meets minimum requirement."""
         try:
             result = subprocess.run(
-                [self.executable_path, "--version"],
+                [str(self.executable_path), "--version"],
                 capture_output=True,
                 text=True,
                 check=True,
@@ -178,10 +212,8 @@ def _convert_single_file(
                 "Use overwrite=True to force."
             )
 
-        # executable_path is guaranteed to be non-None after __init__
-        assert self.executable_path is not None
         cmd = [
-            self.executable_path,
+            str(self.executable_path),
             "--invisible",
             "--headless",
             "--nologo",

src/rtflite/encode.py

@@ -447,7 +447,10 @@ def write_rtf(self, file_path: str | Path) -> None:
         target_path.write_text(rtf_code, encoding="utf-8")
 
     def write_docx(
-        self, file_path: str | Path, executable_path: str | None = None
+        self,
+        file_path: str | Path,
+        *,
+        converter: LibreOfficeConverter | None = None,
     ) -> None:
         """Write the document as a DOCX file.
 
@@ -460,27 +463,39 @@ def write_docx(
             file_path: Destination path for the DOCX file.
                 Accepts string or Path input. Can be absolute or relative.
                 Directories are created if they do not already exist.
-            executable_path: Optional path to the LibreOffice executable.
-                If not provided, the default system path will be used.
+            converter: Optional LibreOffice converter instance.
+                Pass a configured instance (for example with a custom
+                `executable_path`) to control how LibreOffice is invoked and to
+                avoid re-initializing and re-verifying the executable path across
+                multiple conversions. Note that each call to ``convert()`` still
+                starts a new LibreOffice process in headless mode; the process is
+                not kept alive between conversions.
 
         Examples:
             ```python
             doc = RTFDocument(df=data, rtf_title=RTFTitle(text="Report"))
             doc.write_docx("output/report.docx")
             ```
 
+            Custom LibreOffice executable:
+            ```python
+            converter = LibreOfficeConverter(executable_path="/custom/path/to/soffice")
+            doc.write_docx("output/report.docx", converter=converter)
+            ```
+
         Note:
             The method prints the file path to stdout for confirmation.
         """
         target_path = Path(file_path).expanduser()
         target_path.parent.mkdir(parents=True, exist_ok=True)
 
+        if converter is None:
+            converter = LibreOfficeConverter()
         with tempfile.TemporaryDirectory() as tmpdir:
             rtf_path = Path(tmpdir) / f"{target_path.stem}.rtf"
             rtf_code = self.rtf_encode()
             rtf_path.write_text(rtf_code, encoding="utf-8")
 
-            converter = LibreOfficeConverter(executable_path=executable_path)
             with tempfile.TemporaryDirectory() as convert_tmpdir:
                 docx_path = converter.convert(
                     input_files=rtf_path,

tests/test_convert_executable_path.py

@@ -0,0 +1,43 @@
+from __future__ import annotations
+
+from unittest.mock import patch
+
+import pytest
+
+from rtflite.convert import LibreOfficeConverter
+
+
+def test_init_accepts_executable_path_as_path(tmp_path, monkeypatch):
+    dummy_executable = tmp_path / "soffice"
+    dummy_executable.write_text("")
+
+    monkeypatch.setattr(LibreOfficeConverter, "_verify_version", lambda self: None)
+    converter = LibreOfficeConverter(executable_path=dummy_executable)
+
+    assert converter.executable_path == dummy_executable
+
+
+def test_init_resolves_executable_name_via_which(tmp_path, monkeypatch):
+    dummy_executable = tmp_path / "soffice"
+    dummy_executable.write_text("")
+
+    monkeypatch.setattr(LibreOfficeConverter, "_verify_version", lambda self: None)
+    with patch("rtflite.convert.shutil.which", return_value=str(dummy_executable)):
+        converter = LibreOfficeConverter(executable_path="soffice")
+
+    assert converter.executable_path == dummy_executable
+
+
+def test_init_raises_for_missing_explicit_path(tmp_path, monkeypatch):
+    monkeypatch.setattr(LibreOfficeConverter, "_verify_version", lambda self: None)
+    with pytest.raises(FileNotFoundError):
+        LibreOfficeConverter(executable_path=tmp_path / "missing")
+
+
+def test_init_raises_for_missing_command(monkeypatch):
+    monkeypatch.setattr(LibreOfficeConverter, "_verify_version", lambda self: None)
+    with (
+        patch("rtflite.convert.shutil.which", return_value=None),
+        pytest.raises(FileNotFoundError),
+    ):
+        LibreOfficeConverter(executable_path="soffice")

tests/test_write_docx_args.py

@@ -1,5 +1,5 @@
 from pathlib import Path
-from unittest.mock import patch
+from unittest.mock import ANY, MagicMock, patch
 
 import polars as pl
 import pytest
@@ -20,28 +20,52 @@ def sample_document() -> RTFDocument:
 
 
 @patch("rtflite.encode.LibreOfficeConverter")
-def test_write_docx_passes_executable_path(
+def test_write_docx_uses_provided_converter(
     mock_converter_cls, sample_document, tmp_path
 ):
-    """Verify that executable_path is correctly passed to LibreOfficeConverter."""
-    # Setup mock
+    """Verify that `write_docx` uses a provided converter instance."""
+    converter = MagicMock()
+    converter.convert.return_value = Path("dummy.docx")
+
+    output_path = tmp_path / "output.docx"
+
+    with patch("rtflite.encode.shutil.move"):
+        sample_document.write_docx(output_path, converter=converter)
+
+    mock_converter_cls.assert_not_called()
+    converter.convert.assert_called_once_with(
+        input_files=ANY,
+        output_dir=ANY,
+        format="docx",
+        overwrite=True,
+    )
+    kwargs = converter.convert.call_args.kwargs
+    assert isinstance(kwargs["output_dir"], Path)
+    assert isinstance(kwargs["input_files"], Path)
+    assert kwargs["input_files"].name == f"{output_path.stem}.rtf"
+
+
+@patch("rtflite.encode.LibreOfficeConverter")
+def test_write_docx_creates_default_converter(
+    mock_converter_cls, sample_document, tmp_path
+):
+    """Verify that `write_docx` creates a default converter when omitted."""
     mock_instance = mock_converter_cls.return_value
     mock_instance.convert.return_value = Path("dummy.docx")
 
-    # Create dummy docx file that would be moved
     output_path = tmp_path / "output.docx"
 
-    # We need to mock shutil.move as well since the converter is mocked
-    # and won't actually create files
-    with patch("shutil.move"):
-        sample_document.write_docx(
-            output_path, executable_path="/custom/path/to/soffice"
-        )
+    with patch("rtflite.encode.shutil.move"):
+        sample_document.write_docx(output_path)
 
-    # Verify LibreOfficeConverter was initialized with the correct path
-    mock_converter_cls.assert_called_once_with(
-        executable_path="/custom/path/to/soffice"
+    mock_converter_cls.assert_called_once_with()
+    mock_instance.convert.assert_called_once_with(
+        input_files=ANY,
+        output_dir=ANY,
+        format="docx",
+        overwrite=True,
     )
-
-    # Verify convert was called
-    mock_instance.convert.assert_called_once()
+    kwargs = mock_instance.convert.call_args.kwargs
+    assert isinstance(kwargs["output_dir"], Path)
+    assert isinstance(kwargs["input_files"], Path)
+    assert kwargs["input_files"].name == f"{output_path.stem}.rtf"