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

fix: correctly handle local files with colons (:) in name #1452

Merged
merged 9 commits into from
Dec 7, 2023

Conversation

lobis
Copy link
Contributor

@lobis lobis commented Dec 7, 2023

Fixes #1447

@martindurant
Copy link
Member

Looks like fsspec.generic.py:_resolve_fs should not pass a list to split_protocol

@lobis
Copy link
Contributor Author

lobis commented Dec 7, 2023

@martindurant looks like the CI is failing due to https://github.com/fsspec/filesystem_spec/blob/master/fsspec/core.py#L516 being called with a list argument. This method should only take strings, so I guess this exposed another issue. Being called with a list was previously bypassing the if "://" in urlpath comparison as it's not comparing with the string contents of the list but with the list itself.

@lobis
Copy link
Contributor Author

lobis commented Dec 7, 2023

Apparently windows does not support : in paths (makes sense I guess). I'll skip the test for windows.

@lobis
Copy link
Contributor Author

lobis commented Dec 7, 2023

I'll double check in our end that this fixes things and I'll mark this PR as ready.

Update: Looks good!

@lobis lobis marked this pull request as ready for review December 7, 2023 17:24
@lobis lobis requested a review from martindurant December 7, 2023 17:25
fsspec/generic.py Outdated Show resolved Hide resolved
fsspec/generic.py Outdated Show resolved Hide resolved
fsspec/generic.py Outdated Show resolved Hide resolved
fsspec/generic.py Outdated Show resolved Hide resolved
@martindurant martindurant merged commit 7c3408a into fsspec:master Dec 7, 2023
10 checks passed
@martindurant
Copy link
Member

It being Thursday evening, a release this week is unlikely.

@lgray
Copy link
Contributor

lgray commented Dec 7, 2023

Could a release come before Dec 13th? That is what matters for decision making.

@martindurant
Copy link
Member

before Dec 13th

Yes, I guarantee it. Ping me on Monday if I haven't done it yet.

@lgray
Copy link
Contributor

lgray commented Dec 11, 2023

@martindurant ping!

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.

Local files with colons in name no longer readable after 2023.12.0 version
3 participants