aai-institute / lakefs-spec

An fsspec implementation for the lakeFS project.
http://lakefs-spec.org/
Apache License 2.0
37 stars 4 forks source link

Add overloads for `LakeFSFileSystem.ls()` #221

Closed nicholasjng closed 6 months ago

nicholasjng commented 6 months ago

Specializes the return type as list[dict[str, Any]] for detail=True, and list[str] for detail=False.

Unfortunately, since the base class has a keyword argument default, the Literal[False] overload is still marked by the IDE.

Furthermore, the typing is changed to list[str] | list[dict[str, Any]], since that is only equivalent to list[str | dict[str, Any]] in case of a homogeneous list. This is our case, but the former list union type explicitly conveys type homogeneity.

nicholasjng commented 6 months ago

Repro:

from lakefs_spec import LakeFSFileSystem

def main():
    fs = LakeFSFileSystem()
    resource = "lakefs-spec-tests/main/data/"
    res = fs.ls(resource)
    res2 = fs.ls(resource, detail=False)

if __name__ == "__main__":
    main()

On the PR branch, res2 is hinted as list[str], which is nicer than what we had before. Unfortunately, due to the absence of these hints on the base class, my Pycharm still complains on overload 2 due to no default argument.

EDIT: More links about return type specialization on booleans: https://github.com/python/mypy/issues/6113 https://github.com/python/mypy/issues/8634

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (e05103d) 90.05% compared to head (7f29721) 90.05%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #221 +/- ## ======================================= Coverage 90.05% 90.05% ======================================= Files 7 7 Lines 533 533 Branches 85 85 ======================================= Hits 480 480 Misses 27 27 Partials 26 26 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.