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

Make S3FileSystem.protocol type tuple[str, ...] #812

Merged
merged 1 commit into from
Oct 21, 2023

Conversation

ap--
Copy link
Contributor

@ap-- ap-- commented Oct 20, 2023

Hello,

this PR makes S3FileSystem more spec compliant by changing the protocol type from list[str] to tuple[str, ...] as required by fsspec:

https://github.com/fsspec/filesystem_spec/blob/6aa8a9a888f8d7ce2c721f6896f1ffabde84ef05/fsspec/spec.py#L107

This caused an issue when I was using s3fs together with ReferenceFileSystem. See fsspec/filesystem_spec#1395

Let me know if I should also add some sort of test to ensure the protocol type.

Cheers,
Andreas

@martindurant
Copy link
Member

This is fine for now, thank you. In the future, we should probably make more tests in https://github.com/fsspec/filesystem_spec/tree/master/fsspec/tests/abstract that ensure the spec is followed for all implementations.

@martindurant martindurant merged commit 60d374a into fsspec:main Oct 21, 2023
5 checks passed
@ap-- ap-- deleted the protocol-tuple branch October 21, 2023 22:23
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