fsspec / fsspec/filesystem_spec

Some implementations don't have same function signatures as base class

Open
#1,100 6 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
1.4k
Forks
490
Avg merge
2d 3h
Merged PRs (30d)
38

Description

Hi, I noticed some implementations change the default value of some parameters, for example: AbstractFilesystem have detail=True in ls, however LocalFileSystem have detail=False. (https://github.com/fsspec/filesystem_spec/blob/master/fsspec/spec.py#L301, https://github.com/fsspec/filesystem_spec/blob/master/fsspec/implementations/local.py#L56)
This is inconsistent, for example, people may expect all filesystems to behave the same in ls operations. Also, as listdir just calls ls, if implementations changed the signature of ls, calling it with listdir will use the default value specified in AbstractFileSystem since it's defined there and implementations don't rewrite that. This behavior inconsistency already caused issues (for example, Lightning-AI/lightning#3805).

So I wrote a test to find out if there is any more inconsistency, and I found a lot. (https://github.com/leoleoasd/filesystem_spec/blob/master/fsspec/implementations/tests/test_common.py#L65, https://github.com/leoleoasd/filesystem_spec/actions/runs/3407906000/jobs/5667983849)

Is this a bug, should we fix it? The fix is definitely a breaking change and may cause some code to stop working. Maybe we should, first, find what implementation has different signatures, and warn users that use its default value about the change?

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Review the signatures in fsspec/spec.py and fsspec/implementations/local.py, then inspect the consistency test in fsspec/implementations/tests/test_common.py. Compare the implementations identified by that test and determine the compatibility policy for differing defaults. Done should include an agreed scope for handling the breaking change and corresponding test coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.