search

fsspec / filesystem_spec

A specification that python filesystems should adhere to.
BSD 3-Clause "New" or "Revised" License
990 stars 347 forks source link

fs.cat_files signature consistency #1600

Open nsmith- opened 4 months ago

nsmith- commented 4 months ago

In debugging https://github.com/scikit-hep/uproot5/pull/1198 I found I was assuming all implementations fs.cat_files had the same signature. In fact they do not,

>>> help(fsspec.filesystem("s3").cat_file)
Help on function _cat_file in module s3fs.core:

_cat_file(path, version_id=None, start=None, end=None)
...
>>> help(fsspec.filesystem("http").cat_file)
Help on function _cat_file in module fsspec.implementations.http:

_cat_file(url, start=None, end=None, **kwargs)
...
>>> help(fsspec.filesystem("file").cat_file)
Help on method cat_file in module fsspec.spec:

cat_file(path, start=None, end=None, **kwargs) method of fsspec.implementations.local.LocalFileSystem instance

etc. and it was my mistake to not notice version_id is ahead of start and end for the s3 filesystem. I think it might be nice to enforce all signatures to require explicit keywords for all arguments after path, e.g.

def cat_file(self, path, *, start=None, end=None, **kwargs): ...

per https://peps.python.org/pep-3102/

martindurant commented 4 months ago

You are welcome to propose such a change to AbstractFileSystem, or just to change the ordering of the one deviant call. I don't know that there would be much opinion on the matter among users, but maybe.