Skip to content

Fix ArrowFSWrapper parent paths for S3#2072

Merged
martindurant merged 2 commits into
fsspec:masterfrom
Sanjays2402:fix/arrow-s3-root-marker
Jul 17, 2026
Merged

Fix ArrowFSWrapper parent paths for S3#2072
martindurant merged 2 commits into
fsspec:masterfrom
Sanjays2402:fix/arrow-s3-root-marker

Conversation

@Sanjays2402

Copy link
Copy Markdown
Contributor

ArrowFSWrapper always inherited / as its root marker, so put() derived /bucket/... parents that pyarrow's S3 backend rejects. S3 wrappers now use an empty instance root marker and resolve parents from that marker; other Arrow backends keep their existing behavior.

Fixes #2015

Tested with the Arrow, spec, core, and mapping test modules (284 passed, 69 skipped, 1 xfailed) and Ruff.

ArrowFSWrapper inherited the "/" root marker for every pyarrow backend,
so put() sent S3 create_dir() a path beginning with a separator.

Use an instance-specific empty marker for S3 and resolve parent paths with it.
Add a regression covering the path passed to the wrapped backend.

Fixes fsspec#2015
@martindurant

Copy link
Copy Markdown
Member

This specific issue was seen before - can you please have a search in the issues/PR? I thought it had been solved...

@Sanjays2402

Copy link
Copy Markdown
Contributor Author

@martindurant I found the earlier work: #1464 and the closed #1467. That PR used a dynamic subclass plus from_uri() probing. Your follow-up commit b129860 moved toward instance-aware handling, but it was never merged and also changed _strip_protocol() plus unrelated compression handling.

This PR is narrower: it sets an instance root_marker only for the S3 backend and overrides only _parent(), preserving the class-level _strip_protocol() behavior and every non-S3 backend. Issue #1464 is still open. If you would rather generalize backend detection instead of checking type_name == "s3", I can revise it that way.

Comment thread fsspec/implementations/arrow.py Outdated
return super()._parent(path)

path = self._strip_protocol(path).lstrip("/")
return path.rsplit("/", 1)[0] if "/" in path else self.root_marker

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

root_maker is guaranteed to be "", no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. In the empty-root branch it is guaranteed to be ""; 971351d now returns the literal directly.

Comment thread fsspec/implementations/arrow.py Outdated
global PYARROW_VERSION
PYARROW_VERSION = get_package_version_without_import("pyarrow")
self.fs = fs
if fs.type_name == "s3":

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is s3 the only exception?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. GCS has the same key-style path semantics; the earlier #1467 regression matrix covered both S3 and GCS. 971351d now selects an empty root marker for both and parameterizes the regression over both backends.

@martindurant
martindurant merged commit 8557062 into fsspec:master Jul 17, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ArrowFSWrapper with S3 does not like leading slash

2 participants