samredai commented on a change in pull request #4285:
URL: https://github.com/apache/iceberg/pull/4285#discussion_r822235103
##########
File path: python/setup.py
##########
@@ -19,7 +19,14 @@
setup(
name="py-iceberg",
- install_requires=[],
+ install_requires=[
Review comment:
These should go in setup.cfg under `[options]`. Did you get these from
the dependencies from the legacy python client's setup.py? I think we should
incrementally add dependencies as PR's require them. We also need to determine
which are optional dependencies and how to group them. For now I think pyarrow
is the only missing dependency:
*setup.cfg*
```
[options]
...
install_requires =
pyarrow
```
As later PRs introduce optional dependencies, we can add them like so:
```
[options.extras_require]
boto =
boto
hive =
hmsclient==0.1.1
```
##########
File path: python/src/iceberg/expressions.py
##########
@@ -53,7 +54,7 @@ class Operation(Enum):
AND = auto()
OR = auto()
- def negate(self):
+ def negate(self) -> Operation:
Review comment:
Can you change `-> Operation` to `-> "Operation"`? That would allow you
to remove the `from __future__ import annotations` at the top.
##########
File path: python/src/iceberg/io/pyarrow.py
##########
@@ -43,9 +43,14 @@ class PyArrowFile(InputFile, OutputFile):
Examples:
>>> from iceberg.io.pyarrow import PyArrowFile
>>> input_file = PyArrowFile("s3://foo/bar.txt")
- >>> file_content = input_file.open().read() # Read the contents of
the PyArrowFile instance
+ >>> # Read the contents of the PyArrowFile instance
+ >>> # Make sure that you have permissions to read/write
+ >>> # file_content = input_file.open().read()
Review comment:
This makes sense for now to get the checkdocs test passing but we talked
about either mocking out S3 with some conftest monkeypatches or using a minio
instance for more robustness. I'm not exactly sure if there's a way to inject
mocks into checkdocs.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
