kevinjqliu commented on code in PR #1860:
URL: https://github.com/apache/iceberg-python/pull/1860#discussion_r2019955353
##########
tests/test_types.py:
##########
@@ -231,6 +246,27 @@ def test_nested_field() -> None:
assert "validation errors for NestedField" in str(exc_info.value)
+def test_nested_field_type_as_str_unsupported() -> None:
+ unsupported_types = ["list", "map", "struct"]
+ for type_str in unsupported_types:
+ with pytest.raises(ValueError) as exc_info:
+ _ = NestedField(1, "field", type_str, required=True)
+ assert f"Unsupported field type: '{type_str}'" in str(exc_info.value)
+
+
+@pytest.mark.parametrize("type_str, type_class", primitive_types.items())
+def test_nested_field_type_as_str(type_str: str, type_class: type) -> None:
Review Comment:
nit: parametrizing the test actually is slower compared to just running a
for loop into the function
```
for type_str, type_class in primitive_types.items():
field_var = NestedField(
1,
"field",
type_str,
required=True,
)
assert isinstance(
field_var.field_type, type_class
), f"Expected {type_class.__name__}, got
{field_var.field_type.__class__.__name__}"
```
##########
tests/test_types.py:
##########
@@ -231,6 +246,27 @@ def test_nested_field() -> None:
assert "validation errors for NestedField" in str(exc_info.value)
+def test_nested_field_type_as_str_unsupported() -> None:
+ unsupported_types = ["list", "map", "struct"]
+ for type_str in unsupported_types:
+ with pytest.raises(ValueError) as exc_info:
+ _ = NestedField(1, "field", type_str, required=True)
+ assert f"Unsupported field type: '{type_str}'" in str(exc_info.value)
+
+
+@pytest.mark.parametrize("type_str, type_class", primitive_types.items())
+def test_nested_field_type_as_str(type_str: str, type_class: type) -> None:
Review Comment:
nit:
```suggestion
def test_nested_field_primitive_type_as_str(type_str: str, type_class: type)
-> None:
```
##########
tests/test_types.py:
##########
@@ -231,6 +246,27 @@ def test_nested_field() -> None:
assert "validation errors for NestedField" in str(exc_info.value)
+def test_nested_field_type_as_str_unsupported() -> None:
+ unsupported_types = ["list", "map", "struct"]
+ for type_str in unsupported_types:
+ with pytest.raises(ValueError) as exc_info:
+ _ = NestedField(1, "field", type_str, required=True)
+ assert f"Unsupported field type: '{type_str}'" in str(exc_info.value)
+
+
+@pytest.mark.parametrize("type_str, type_class", primitive_types.items())
+def test_nested_field_type_as_str(type_str: str, type_class: type) -> None:
+ field_var = NestedField(
+ 1,
+ "field",
+ type_str,
+ required=True,
+ )
+ assert isinstance(
+ field_var.field_type, type_class
+ ), f"Expected {type_class.__name__}, got
{field_var.field_type.__class__.__name__}"
+
Review Comment:
nit can we add a test for this case too?
```
except ValueError as e:
raise ValueError(f"Unsupported field type: '{v}'") from e
```
##########
tests/test_types.py:
##########
@@ -231,6 +246,27 @@ def test_nested_field() -> None:
assert "validation errors for NestedField" in str(exc_info.value)
+def test_nested_field_type_as_str_unsupported() -> None:
Review Comment:
nit:
```suggestion
def test_nested_field_complex_type_as_str_unsupported() -> None:
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
