HonahX commented on code in PR #504:
URL: https://github.com/apache/iceberg-python/pull/504#discussion_r1515657781


##########
pyiceberg/table/__init__.py:
##########
@@ -294,17 +295,21 @@ def upgrade_table_version(self, format_version: 
Literal[1, 2]) -> Transaction:
 
         return self
 
-    def set_properties(self, **updates: str) -> Transaction:
+    def set_properties(self, properties: Properties = EMPTY_DICT, **kwargs: 
Any) -> Transaction:

Review Comment:
   Just copy my comment here 
https://github.com/apache/iceberg-python/issues/502#issuecomment-1982550509
   
   



##########
pyiceberg/table/__init__.py:
##########
@@ -469,7 +474,9 @@ class SetLocationUpdate(TableUpdate):
 
 class SetPropertiesUpdate(TableUpdate):
     action: TableUpdateAction = TableUpdateAction.set_properties
-    updates: Dict[str, str]
+    updates: Properties
+    # validators
+    transform_properties_dict_value_to_str = field_validator('updates', 
mode='before')(transform_dict_value_to_str)

Review Comment:
   ```suggestion
       @field_validator('updates', mode='before')
       def transform_properties_dict_value_to_str(cls, properties: Properties) 
-> Dict[str, str]:
           return transform_dict_value_to_str(properties)
   ```
   Shall we reformat the validator to use decorator? This aligns with the way 
we define model validators in `metadata.py` and also the example in [Pydantic 
doc](https://docs.pydantic.dev/latest/concepts/validators/#field-validators). 
WDYT?



-- 
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

Reply via email to