rambleraptor commented on code in PR #3889:
URL: https://github.com/apache/iceberg-python/pull/3889#discussion_r3928060667
##########
pyiceberg/io/pyarrow.py:
##########
@@ -393,6 +393,13 @@ def to_input_file(self) -> PyArrowFile:
return self
+def _require_pyarrow_version(min_version: str, feature: str) -> None:
Review Comment:
Idea (this is out-of-scope): What if we had a
`require_pyarrow_with_version` method that checks if pyarrow exists and
optionally checks the version?
We do pyarrow checks across the codebase.
##########
pyiceberg/io/pyarrow.py:
##########
@@ -535,14 +542,7 @@ def _initialize_s3_fs(self, netloc: str | None) ->
FileSystem:
def _initialize_azure_fs(self) -> FileSystem:
#
https://arrow.apache.org/docs/python/generated/pyarrow.fs.AzureFileSystem.html
- from packaging import version
-
- MIN_PYARROW_VERSION_SUPPORTING_AZURE_FS = "20.0.0"
- if version.parse(pyarrow.__version__) <
version.parse(MIN_PYARROW_VERSION_SUPPORTING_AZURE_FS):
- raise ImportError(
- f"pyarrow version >= {MIN_PYARROW_VERSION_SUPPORTING_AZURE_FS}
required for AzureFileSystem support, "
- f"but found version {pyarrow.__version__}."
- )
+ _require_pyarrow_version("20.0.0", "AzureFileSystem support")
Review Comment:
I love this.
##########
pyiceberg/io/pyarrow.py:
##########
@@ -2959,6 +2959,37 @@ def _get_parquet_writer_kwargs(table_properties:
Properties) -> dict[str, Any]:
),
}
+ # Unlike the properties above, which PyArrow's writer never supports and
are safe to silently
Review Comment:
Can we simplify or remove this comment? It doesn't seem terribly useful.
--
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]
