paulcichonski commented on code in PR #1432:
URL: https://github.com/apache/iceberg-python/pull/1432#discussion_r1887299466
##########
pyiceberg/types.py:
##########
@@ -328,8 +328,8 @@ def __init__(
data["type"] = data["type"] if "type" in data else field_type
data["required"] = required
data["doc"] = doc
Review Comment:
That could work, my only worry would be that ordinarily minor changes like
changing field order in the class definition would break consumers in
unexpected ways.
For example, this instantiation works fine:
```python
from pyiceberg.types import NestedField, StringType
>>> NestedField(1, 'test', StringType(), True)
NestedField(field_id=1, name='test', field_type=StringType(), required=True)
```
If someone (for some reason) changed field order or adds a new field in the
middle, for example:
```diff
diff --git a/pyiceberg/types.py b/pyiceberg/types.py
index bd0eb7a..f17a3d4 100644
--- a/pyiceberg/types.py
+++ b/pyiceberg/types.py
@@ -304,33 +304,22 @@ class NestedField(IcebergType):
field_id: int = Field(alias="id")
name: str = Field()
- field_type: SerializeAsAny[IcebergType] = Field(alias="type")
required: bool = Field(default=False)
+ field_type: SerializeAsAny[IcebergType] = Field(alias="type")
```
Then the instantiation breaks:
```python
>>> NestedField(1, 'test', StringType(), True)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File
"/Users/pcichons/dev/code/cisco-sbgidm/iceberg-python/pyiceberg/types.py", line
322, in __init__
super().__init__(**kwargs)
File
"/Users/pcichons/dev/code/cisco-sbgidm/iceberg-python/.venv/lib/python3.12/site-packages/pydantic/main.py",
line 214, in __init__
validated_self = self.__pydantic_validator__.validate_python(data,
self_instance=self)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
pydantic_core._pydantic_core.ValidationError: 2 validation errors for
NestedField
required
Input should be a valid boolean [type=bool_type, input_value=StringType(),
input_type=StringType]
For further information visit
https://errors.pydantic.dev/2.10/v/bool_type
field_type
Input should be a valid dictionary or instance of IcebergType
[type=model_type, input_value=True, input_type=bool]
For further information visit
https://errors.pydantic.dev/2.10/v/model_type
```
Tests could catch that, but it might get a bit painful to deal with.
--
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
