You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by fo...@apache.org on 2026/07/27 18:40:25 UTC

(iceberg-python) branch main updated: Convert `FileFormatModel` from `ABC` to `typing.Protocol` (#3692)

This is an automated email from the ASF dual-hosted git repository.

Fokko pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git


The following commit(s) were added to refs/heads/main by this push:
     new 154288fb7 Convert `FileFormatModel` from `ABC` to `typing.Protocol` (#3692)
154288fb7 is described below

commit 154288fb71a601b653804778f45e5781224038df
Author: Neelesh Salian <ns...@users.noreply.github.com>
AuthorDate: Mon Jul 27 11:40:19 2026 -0700

    Convert `FileFormatModel` from `ABC` to `typing.Protocol` (#3692)
    
    Closes #3100
    
    ## Rationale for this change
    
    `FileFormatModel` had only abstract methods (no shared implementation),
    so an `ABC` was heavier than needed. This PR converts it to
    `typing.Protocol`, which is the more Pythonic pattern for a pure
    interface. `FileFormatWriter` remains an `ABC` since it has shared
    `__enter__`/`__exit__`/`result` implementations that inheritance carries
    for free. `FileFormatFactory` is unchanged - it matches the existing
    `AuthManagerFactory` pattern.
    
    ## Changes
    
    New format implementations can now conform structurally without
    inheriting from `FileFormatModel`. Net: 2 `ABC`s → 1 `ABC` + 1
    `Protocol`. No signature change for existing callers;
    `ParquetFormatModel(FileFormatModel)` inheritance is retained
    (`Protocol` supports explicit conformance).
    
    ## Are these changes tested?
    
    Yes. `tests/io/test_fileformat.py` adds two tests using a shared
    `_StructuralModel` helper:
    
    - `test_file_format_model_is_protocol` - a structurally-conforming class
    (no inheritance) passes `isinstance()` against `FileFormatModel`.
    - `test_structural_model_works_with_factory` - a structurally-conforming
    class can be registered and retrieved via `FileFormatFactory`
    end-to-end.
    
    ## Are there any user-facing changes?
    
    No. Default behavior is unchanged.
---
 pyiceberg/io/fileformat.py  | 11 +++++------
 tests/io/test_fileformat.py | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/pyiceberg/io/fileformat.py b/pyiceberg/io/fileformat.py
index 337e69860..65c0cbeff 100644
--- a/pyiceberg/io/fileformat.py
+++ b/pyiceberg/io/fileformat.py
@@ -21,7 +21,7 @@ from __future__ import annotations
 
 from abc import ABC, abstractmethod
 from dataclasses import dataclass
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING, Any, Protocol, runtime_checkable
 
 from pyiceberg.io import OutputFile
 from pyiceberg.manifest import FileFormat
@@ -143,18 +143,17 @@ class FileFormatWriter(ABC):
         self._result = self.close()
 
 
-class FileFormatModel(ABC):
+@runtime_checkable
+class FileFormatModel(Protocol):
     """Represents a file format's capabilities. Creates writers."""
 
     @property
-    @abstractmethod
     def format(self) -> FileFormat: ...
 
-    @abstractmethod
     def file_extension(self) -> str:
         """Return file extension without dot, e.g. 'parquet', 'orc'."""
+        ...
 
-    @abstractmethod
     def create_writer(
         self,
         output_file: OutputFile,
@@ -162,9 +161,9 @@ class FileFormatModel(ABC):
         properties: Properties,
     ) -> FileFormatWriter: ...
 
-    @abstractmethod
     def add_field_metadata(self, field: NestedField, metadata: dict[bytes, bytes], include_field_ids: bool) -> None:
         """Add format-specific Arrow field metadata."""
+        ...
 
 
 class FileFormatFactory:
diff --git a/tests/io/test_fileformat.py b/tests/io/test_fileformat.py
index 328fee927..d5d487fa6 100644
--- a/tests/io/test_fileformat.py
+++ b/tests/io/test_fileformat.py
@@ -77,3 +77,36 @@ def test_result_before_close_raises() -> None:
     writer = _DummyWriter()
     with pytest.raises(RuntimeError, match="Writer has not been closed yet"):
         writer.result()
+
+
+class _StructuralModel:
+    """Non-inheriting class that structurally conforms to the FileFormatModel Protocol."""
+
+    @property
+    def format(self) -> FileFormat:
+        return FileFormat.ORC
+
+    def file_extension(self) -> str:
+        return "orc"
+
+    def create_writer(self, output_file: Any, file_schema: Any, properties: Any) -> Any:
+        raise NotImplementedError
+
+    def add_field_metadata(self, field: Any, metadata: Any, include_field_ids: bool) -> None:
+        pass
+
+
+def test_file_format_model_is_protocol() -> None:
+    """A structurally-conforming class (no inheritance) passes isinstance() against FileFormatModel."""
+    assert isinstance(_StructuralModel(), FileFormatModel)
+
+
+def test_structural_model_works_with_factory() -> None:
+    """A structurally-conforming class (no inheritance) can be registered and retrieved via FileFormatFactory."""
+    original = dict(FileFormatFactory._registry)
+    try:
+        model = _StructuralModel()
+        FileFormatFactory.register(model)
+        assert FileFormatFactory.get(FileFormat.ORC) is model
+    finally:
+        FileFormatFactory._registry = original