From eb57123e0d98a2095d499b4ba7c9b09d30945ec2 Mon Sep 17 00:00:00 2001 From: TiM Date: Tue, 1 Sep 2026 08:34:49 +1200 Subject: [PATCH] Mirror `readinto` from the pyarrow stream onto `ArrowFile` `ArrowFile` mirrors a fixed method list from the pyarrow stream it wraps and `readinto` is not on it, though `pyarrow.NativeFile` implements it. That leaves it the one file class here without one, so code written against the `AbstractBufferedFile` contract breaks on an Arrow-backed filesystem and nowhere else. `LibArchiveFileSystem` is the in-repo case, and it fails silently. Its `custom_reader` calls `readinto` from a ctypes callback, which swallows the `AttributeError`, so libarchive sees a zero-length read and `ls()` returns an empty archive instead of raising. Buffered reads at least raise: a sized `read`, `peek` and `read1` each fill the buffer through `readinto`, while an unsized `read()` falls back to `read(-1)` and is unaffected. Gzip is how it turned up. `fsspec.compression` registers isal's `IGzipFile` as the `gzip` codec when `isal` imports, and that reader decompresses through `readinto`, so reading a `.gz` file through an Arrow-backed filesystem fails in an environment that happens to carry the package and reads fine everywhere else. --- fsspec/implementations/arrow.py | 1 + fsspec/implementations/tests/test_arrow.py | 36 ++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/fsspec/implementations/arrow.py b/fsspec/implementations/arrow.py index c312cd4a5..d17351441 100644 --- a/fsspec/implementations/arrow.py +++ b/fsspec/implementations/arrow.py @@ -227,6 +227,7 @@ def get_file(self, rpath, lpath, **kwargs): "stream", [ "read", + "readinto", "seek", "tell", "write", diff --git a/fsspec/implementations/tests/test_arrow.py b/fsspec/implementations/tests/test_arrow.py index 52033c938..acbfea379 100644 --- a/fsspec/implementations/tests/test_arrow.py +++ b/fsspec/implementations/tests/test_arrow.py @@ -1,3 +1,4 @@ +import io import secrets import pytest @@ -271,6 +272,41 @@ def test_seekable(fs, remote_dir): file.seek(5) +def test_readinto(fs, remote_dir): + data = b"dvc.org" + + with fs.open(remote_dir + "/a.txt", "wb") as stream: + stream.write(data) + + for seekable in [True, False]: + with fs.open(remote_dir + "/a.txt", "rb", seekable=seekable) as file: + buffer = bytearray(3) + assert file.readinto(buffer) == 3 + assert bytes(buffer) == data[:3] + + +@pytest.mark.parametrize( + "read", + [ + lambda buffered: buffered.read(3), + lambda buffered: buffered.peek(3)[:3], + lambda buffered: buffered.read1(3), + ], + ids=["read", "peek", "read1"], +) +def test_readinto_supports_a_buffered_reader(fs, remote_dir, read): + # Each of these fills a buffer io.BufferedReader owns, so they go through + # readinto rather than read(); an unsized read() does not. A gzip reader takes + # the peek()/read1() path. + data = b"dvc.org" + + with fs.open(remote_dir + "/a.txt", "wb") as stream: + stream.write(data) + + with fs.open(remote_dir + "/a.txt", "rb") as file: + assert read(io.BufferedReader(file)) == data[:3] + + def test_get_kwargs_from_urls_hadoop_fs(): kwargs = HadoopFileSystem._get_kwargs_from_urls( "hdfs://user@localhost:8020/?replication=2"