Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DirFS: Handle paths with no leading / (#1638) #1639

Merged
merged 2 commits into from
Jun 29, 2024

Conversation

metadaddy
Copy link
Contributor

@metadaddy metadaddy commented Jun 27, 2024

If you wrap an S3FileSystem with DirFileSystem, ls() breaks, since DirFileSystem assumes that the underlying FS returns paths with a leading /, but S3FileSystem does not (see #1638).

This change to _relpath() removes the leading / from the prefix used in constructing the relative path when the incoming path has no leading /, but the DirFileSystem path does.

@metadaddy metadaddy force-pushed the fix-leading-slash branch 2 times, most recently from 0f1013a to 814369e Compare June 27, 2024 21:50
@martindurant
Copy link
Member

Thanks for sorting this out.
It sort of feels untidy, but I can't think of a better way to do this :)

@martindurant martindurant merged commit 7fee7c1 into fsspec:master Jun 29, 2024
11 checks passed
@metadaddy
Copy link
Contributor Author

Yeah - I spent a while looking at DirFS and S3FS before concluding that this was the way. The root cause, I think, is that fsspec itself doesn't specify whether or not a leading slash is required/expected, while DirFS assumes it will be present.

Thanks for fixing the lint nit!

@metadaddy metadaddy deleted the fix-leading-slash branch June 29, 2024 22:17
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.

2 participants