amogh-jahagirdar commented on code in PR #411:
URL: https://github.com/apache/iceberg-python/pull/411#discussion_r1485695745


##########
pyiceberg/table/metadata.py:
##########
@@ -411,6 +442,24 @@ def new_table_metadata(
     if table_uuid is None:
         table_uuid = uuid.uuid4()
 
+    # Remove format-version so it does not get persisted
+    format_version = int(properties.pop("format-version", 
DEFAULT_FORMAT_VERSION))

Review Comment:
   We can and probably should but in this PR we can't reference it in 
`metadata.py` since it ends up being a circular reference. 
   
   ```
    ImportError: cannot import name 'TableProperties' from partially 
initialized module 'pyiceberg.table' (most likely due to a circular import) 
(/home/amogh/workspace/iceberg-python/pyiceberg/table/__init__.py
   ```
   
   I think the way to handle that is by moving it to a separate file but still 
ensure that we preserve backwards compatibility for any code which was 
importing TableProperties from the original table namespace.
   
   I think for now we can just have the local constants and have an issue to 
handle the organization of TableProperties in a future PR? Unless I'm missing a 
simple way to address this right here.



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