kevinjqliu commented on code in PR #1533:
URL: https://github.com/apache/iceberg-python/pull/1533#discussion_r1925533289
##########
.python-version:
##########
Review Comment:
this is part of pyenv's local config, should not be checked into the repo
##########
poetry.lock:
##########
Review Comment:
can you rebase this PR against main to get the latest change from #1538?
##########
pyiceberg/table/inspect.py:
##########
@@ -57,7 +87,21 @@ def _get_snapshot(self, snapshot_id: Optional[int] = None)
-> Snapshot:
raise ValueError("Cannot get a snapshot as the table does not have
any.")
def snapshots(self) -> "pa.Table":
- import pyarrow as pa
Review Comment:
i think we actually need this here, in case someone imports this function
directly
```
from pyiceberg.table.inspect import snapshots
```
##########
pyiceberg/table/inspect.py:
##########
@@ -28,12 +29,24 @@
from pyiceberg.utils.singleton import _convert_to_hashable_type
if TYPE_CHECKING:
- import pyarrow as pa
Review Comment:
i think we still need this here
##########
pyiceberg/table/inspect.py:
##########
@@ -95,7 +139,21 @@ def snapshots(self) -> "pa.Table":
)
def entries(self, snapshot_id: Optional[int] = None) -> "pa.Table":
- import pyarrow as pa
+ """ Generate a table of manifest entries for a specific snapshot.
+
+ This method retrieves metadata for all manifest entries within a
given snapshot,
+ including file-level statistics such as column sizes and value counts.
+
+ Args:
+ snapshot_id (Optional[int]): The ID of the snapshot to retrieve
entries for.
+ If None, entries for the current snapshot are returned.
Review Comment:
i like this description of how `snapshot_id` is used, can we apply this for
all similar functions
perhaps something more generic
```suggestion
Args:
snapshot_id (Optional[int]): The ID of the snapshot to retrieve.
If None, the current snapshot is used.
```
##########
mkdocs/mkdocs.yml:
##########
@@ -31,7 +31,8 @@ plugins:
- mkdocstrings:
handlers:
python:
- paths: [..]
+ paths:
+ - pyiceberg
Review Comment:
same for this, what is this change for?
##########
mkdocs/docs/SUMMARY.md:
##########
@@ -30,7 +30,8 @@
- [Verify a release](verify-release.md)
- [How to release](how-to-release.md)
- [Release Notes](https://github.com/apache/iceberg-python/releases)
-- [Code Reference](reference/)
+- [Code Reference](reference/pyiceberg/index.md)
Review Comment:
why was this changed? i dont think this is necessary
--
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
