ENH: use native filesystem (if available) for read_parquet with pyarrow engine #51609
Conversation
This reverts commit 7ec7d75.
pandas/io/parquet.py
Outdated
| try: | ||
| fs_arrow = import_optional_dependency("fsspec.implementations.arrow") | ||
| fs, path_or_handle = pa_fs.FileSystem.from_uri(path) | ||
| fs = fs_arrow.ArrowFSWrapper(fs) |
There was a problem hiding this comment.
Is there a reason we need to wrap it here? Are we using some fsspec APIs? From looking at the code, it seems we just pass this object through to pyarrow as filesystem keyword? In that case, we can (and should) pass the actual pyarrow filesystem .
(in pyarrow, we should probably unwrap such an fsspec filesystem that wraps a pyarrow filesystem, but at the moment we don't do that, we just see it as a generic fsspec filesystem)
There was a problem hiding this comment.
I was thinking if there is a future fix/enhancement that expects the filesystem to be fsspec-like that this would be convenient, but it isn't needed particularly now. We can add that if needed in the future
(I was under the impression that ArrowFSWrapper would be interpreted as an pyarrow filesystem since that's the backend from https://arrow.apache.org/docs/python/filesystems.html#using-arrow-filesystems-with-fsspec)
There was a problem hiding this comment.
(I was under the impression that
ArrowFSWrapperwould be interpreted as an pyarrow filesystem since that's the backend from https://arrow.apache.org/docs/python/filesystems.html#using-arrow-filesystems-with-fsspec)
That section is for if you want to use a pyarrow filesystem in an fsspec context (for example for passing it to a function that expects an fsspec filesystem, or to use the richer API it provives). But in this case we are not in such a context, since we are passing the filesystem to a pyarrow function.
It doesn't really say anything about then using that wrapper in a pyarrow context, but as mentioned above, we can certainly unwrap this inside pyarrow, avoiding the confusion / wrong expectation you just had (but so currently we don't do that yet).
There was a problem hiding this comment.
Opened apache/arrow#34542
pandas/io/parquet.py
Outdated
| @@ -417,6 +456,12 @@ def to_parquet( | |||
|
|
|||
| .. versionadded:: 1.2.0 | |||
|
|
|||
| filesystem : fsspec or pyarrow filesystem, default None | |||
| Filesystem object to use when reading the parquet file. Only implemented | |||
| for ``engine-"pyarrow"``. | |||
There was a problem hiding this comment.
| for ``engine-"pyarrow"``. | |
| for ``engine="pyarrow"``. |
pandas/io/parquet.py
Outdated
| @@ -504,6 +557,12 @@ def read_parquet( | |||
|
|
|||
| .. versionadded:: 2.0.0 | |||
|
|
|||
| filesystem : fsspec or pyarrow filesystem, default None | |||
| Filesystem object to use when reading the parquet file. Only implemented | |||
| for ``engine-"pyarrow"``. | |||
There was a problem hiding this comment.
| for ``engine-"pyarrow"``. | |
| for ``engine="pyarrow"``. |
pandas/tests/io/test_gcs.py
Outdated
| @staticmethod | ||
| def from_uri(path): | ||
| to_local = path.replace("gs", "file") | ||
| return pa_fs.LocalFileSystem.from_uri(to_local)[0], to_local |
There was a problem hiding this comment.
Could we somehow also assert that this is being called? To actually ensure we get here, because if not (and it's still using fsspec), the tests still pass I assume.
(eg define a list that gets appended with some value inside this function, and yield that from this fixture, so we can see that it was appended after calling read_parquet. Not sure if that's worth the complexity)
There was a problem hiding this comment.
Sure. I added a print statement and validated that the message was printed with capsys
pandas/io/parquet.py
Outdated
| ) | ||
| elif pa_fs is not None and isinstance(fs, pa_fs.FileSystem) and storage_options: | ||
| raise NotImplementedError( | ||
| "storage_options not supported with a pyarrow Filesystem." |
There was a problem hiding this comment.
| "storage_options not supported with a pyarrow Filesystem." | |
| "storage_options not supported with a pyarrow FileSystem." |
pandas/io/parquet.py
Outdated
| elif pa_fs is not None and isinstance(fs, pa_fs.FileSystem) and storage_options: | ||
| raise NotImplementedError( | ||
| "storage_options not supported with a pyarrow Filesystem." | ||
| ) | ||
| if is_fsspec_url(path_or_handle) and fs is None: | ||
| fsspec = import_optional_dependency("fsspec") |
There was a problem hiding this comment.
You could move this import a bit lower to where fsspec is used? (so we don't try to import it if we already have a pyarrow filesystem)
|
Thanks! |
Reboot of #41194 cc @jorisvandenbossche