You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/08/07 19:16:52 UTC
[GitHub] [iceberg] samredai opened a new pull request, #5457: Move base.py for catalog to __init__
samredai opened a new pull request, #5457:
URL: https://github.com/apache/iceberg/pull/5457
This is broken out from PR #5360 and moves `catalog/base.py` to `catalog/__init__.py`. and fixes all effected imports. To avoid a circular import, this also removes the definition for `Identifier` and `Properties` to a `typedef.py` file.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] samredai commented on a diff in pull request #5457: Move base.py for catalog to __init__
Posted by GitBox <gi...@apache.org>.
samredai commented on code in PR #5457:
URL: https://github.com/apache/iceberg/pull/5457#discussion_r939711271
##########
python/pyiceberg/typedef.py:
##########
@@ -0,0 +1,20 @@
+# Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
These were moved to this new file to avoid this circular import error:
```py
ImportError while loading conftest '/Users/sam/iceberg/python/tests/conftest.py'.
tests/conftest.py:55: in <module>
from tests.catalog.test_base import InMemoryCatalog
tests/catalog/test_base.py:28: in <module>
from pyiceberg.catalog import (
pyiceberg/catalog/__init__.py:25: in <module>
from pyiceberg.table.base import Table
pyiceberg/table/base.py:23: in <module>
from pyiceberg.catalog import Identifier
E ImportError: cannot import name 'Identifier' from partially initialized module 'pyiceberg.catalog' (most likely due to a circular import) (/Users/sam/iceberg/python/pyiceberg/catalog/__init__.py)
```
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #5457: Move base.py for catalog to __init__
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5457:
URL: https://github.com/apache/iceberg/pull/5457#discussion_r939718043
##########
python/pyiceberg/table/base.py:
##########
@@ -19,8 +19,8 @@
from pydantic import Field
-from pyiceberg.catalog.base import Identifier
from pyiceberg.table.metadata import TableMetadataV1, TableMetadataV2
+from pyiceberg.typedef import Identifier
Review Comment:
@samredai, should we also move this file's contents to either `table.py` or `table/__init__.py`? (In a follow-up)
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] samredai commented on a diff in pull request #5457: Python: Move base.py for catalog to __init__
Posted by GitBox <gi...@apache.org>.
samredai commented on code in PR #5457:
URL: https://github.com/apache/iceberg/pull/5457#discussion_r939719271
##########
python/pyiceberg/table/base.py:
##########
@@ -19,8 +19,8 @@
from pydantic import Field
-from pyiceberg.catalog.base import Identifier
from pyiceberg.table.metadata import TableMetadataV1, TableMetadataV2
+from pyiceberg.typedef import Identifier
Review Comment:
Done! https://github.com/apache/iceberg/pull/5458
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on a diff in pull request #5457: Move base.py for catalog to __init__
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5457:
URL: https://github.com/apache/iceberg/pull/5457#discussion_r939718043
##########
python/pyiceberg/table/base.py:
##########
@@ -19,8 +19,8 @@
from pydantic import Field
-from pyiceberg.catalog.base import Identifier
from pyiceberg.table.metadata import TableMetadataV1, TableMetadataV2
+from pyiceberg.typedef import Identifier
Review Comment:
@samredai, should we also move this to either `table.py` or `table/__init__.py`?
##########
python/pyiceberg/table/base.py:
##########
@@ -19,8 +19,8 @@
from pydantic import Field
-from pyiceberg.catalog.base import Identifier
from pyiceberg.table.metadata import TableMetadataV1, TableMetadataV2
+from pyiceberg.typedef import Identifier
Review Comment:
@samredai, should we also move this file's contents to either `table.py` or `table/__init__.py`?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #5457: Python: Move base.py for catalog to __init__
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5457:
URL: https://github.com/apache/iceberg/pull/5457#issuecomment-1207479352
Thanks, @samredai!
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #5457: Move base.py for catalog to __init__
Posted by GitBox <gi...@apache.org>.
rdblue merged PR #5457:
URL: https://github.com/apache/iceberg/pull/5457
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org