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 open_local returning list for pathlib.Path #1418

Merged
merged 7 commits into from
Nov 9, 2023

Conversation

BENR0
Copy link
Contributor

@BENR0 BENR0 commented Nov 9, 2023

Input of single pathlib.Path to url argument now returns string instead of list similar to input of single str.

Copy link
Member

@martindurant martindurant left a comment

Choose a reason for hiding this comment

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

Thanks for adding comprehensive tests

fsspec/core.py Outdated
def open_local(url, mode="rb", **storage_options):
def open_local(
url: str | list[str] | Path | list[Path],
mode: Optional[Literal["rb"]] = "rb",
Copy link
Member

Choose a reason for hiding this comment

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

This annotation is wrong, I think, it implies mode can be ["rb", None] and nothing else.

Suggested change
mode: Optional[Literal["rb"]] = "rb",
mode: str = "rb",

The actual legal possibilities depend on the target filesystem, but include ["rb", "r", "rt", "w", "wb", "wt"]

@martindurant
Copy link
Member

I think you may need

from __future__ import annotations

to get those type things to import. Or just remove them.

@martindurant martindurant merged commit 4f70f1b into fsspec:master Nov 9, 2023
10 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.

Inconsistent return type in open_local for string and pathlib.Path
2 participants