adopt review feedback

Author: sungwy

pyiceberg/types.py

@@ -33,7 +33,6 @@
 from __future__ import annotations
 
 import re
-from enum import IntEnum
 from functools import cached_property
 from typing import (
     Any,
@@ -54,7 +53,7 @@
 from pydantic_core.core_schema import ValidatorFunctionWrapHandler
 
 from pyiceberg.exceptions import ValidationError
-from pyiceberg.typedef import IcebergBaseModel, IcebergRootModel, L
+from pyiceberg.typedef import IcebergBaseModel, IcebergRootModel, L, TableVersion
 from pyiceberg.utils.parsing import ParseNumberFromBrackets
 from pyiceberg.utils.singleton import Singleton
 
@@ -63,12 +62,6 @@
 FIXED_PARSER = ParseNumberFromBrackets(FIXED)
 
 
-class TableVersion(IntEnum):
-    ONE = 1
-    TWO = 2
-    THREE = 3
-
-
 def transform_dict_value_to_str(dict: Dict[str, Any]) -> Dict[str, str]:
     """Transform all values in the dictionary to string. Raise an error if any value is None."""
     for key, value in dict.items():
@@ -190,7 +183,7 @@ def is_struct(self) -> bool:
 
     def minimum_format_version(self) -> TableVersion:
         """Minimum Iceberg format version after which this type is supported."""
-        return TableVersion.ONE
+        return 1
 
 
 class PrimitiveType(Singleton, IcebergRootModel[str], IcebergType):
@@ -734,7 +727,7 @@ class TimestampNanoType(PrimitiveType):
     root: Literal["timestamp_ns"] = Field(default="timestamp_ns")
 
     def minimum_format_version(self) -> TableVersion:
-        return TableVersion.THREE
+        return 3
 
 
 class TimestamptzNanoType(PrimitiveType):
@@ -753,7 +746,7 @@ class TimestamptzNanoType(PrimitiveType):
     root: Literal["timestamptz_ns"] = Field(default="timestamptz_ns")
 
     def minimum_format_version(self) -> TableVersion:
-        return TableVersion.THREE
+        return 3
 
 
 class StringType(PrimitiveType):
@@ -820,4 +813,4 @@ class UnknownType(PrimitiveType):
     root: Literal["unknown"] = Field(default="unknown")
 
     def minimum_format_version(self) -> TableVersion:
-        return TableVersion.THREE
+        return 3

tests/integration/test_reads.py

@@ -1008,6 +1008,7 @@ def test_read_unknown_type(catalog: Catalog) -> None:
     tbl = catalog.create_table(
         identifier,
         schema=arrow_table.schema,
+        properties={"format-version": "3"},
     )
 
     tbl.append(arrow_table)

tests/table/test_metadata.py

@@ -48,10 +48,12 @@
     LongType,
     MapType,
     NestedField,
+    PrimitiveType,
     StringType,
     StructType,
     TimestampNanoType,
     TimestamptzNanoType,
+    UnknownType,
 )
 
 
@@ -843,9 +845,17 @@ def test_new_table_metadata_with_v3_schema() -> None:
     assert actual.sort_orders == [expected_sort_order]
 
 
-def test_new_table_metadata_format_v2_with_v3_schema_fails() -> None:
+@pytest.mark.parametrize(
+    "field_type",
+    [
+        TimestampNanoType(),
+        TimestamptzNanoType(),
+        UnknownType(),
+    ],
+)
+def test_new_table_metadata_format_v2_with_v3_schema_fails(field_type: PrimitiveType) -> None:
     schema = Schema(
-        NestedField(field_id=34, name="qux", field_type=TimestampNanoType(), required=False),
+        NestedField(field_id=34, name="qux", field_type=field_type, required=False),
         schema_id=10,
     )
 
@@ -858,7 +868,7 @@ def test_new_table_metadata_format_v2_with_v3_schema_fails() -> None:
         order_id=34,
     )
 
-    with pytest.raises(ValueError, match="timestamp_ns is only supported in 3 or higher. Current format version is: 2"):
+    with pytest.raises(ValueError, match=f"{field_type} is only supported in 3 or higher. Current format version is: 2"):
         new_table_metadata(
             schema=schema,
             partition_spec=partition_spec,