search

rapidsai / dependency-file-generator

https://pypi.org/project/rapids-dependency-file-generator/
Apache License 2.0
15 stars 13 forks source link

fix: fix type hints for delete_existing_files() #96

Closed jameslamb closed 3 months ago

jameslamb commented 3 months ago

Contributes to #87

Resolves these errors from mypy:

_rapids_dependency_file_generator.py:32: error: Value of type variable "AnyStr" of "walk" cannot be "Union[PathLike[Any], Any]"  [type-var]
_rapids_dependency_file_generator.py:33: error: Need type annotation for "fn"  [var-annotated]
_rapids_dependency_file_generator.py:33: error: Argument 1 to "filter" has incompatible type "Callable[[Any], Any]"; expected "Callable[[Union[PathLike[Any], Any]], TypeGuard[Never]]"  [arg-type]
_rapids_dependency_file_generator.py:33: error: Item "PathLike[Any]" of "Union[PathLike[Any], Any]" has no attribute "endswith"  [union-attr]

os.PathLike is not intended to be used as a generic type hint wherever you might have a string that represents a filepath. It has a different, very specific purpose. From https://docs.python.org/3/library/os.html#os.PathLike

An abstract base class for objects representing a file system path, e.g. pathlib.PurePath.

delete_existing_files() is always passed a Python string, so it's type hint should be str.

https://github.com/rapidsai/dependency-file-generator/blob/6b0247a23556bf4cc5f5093864cfafd3edb1c363/src/rapids_dependency_file_generator/_cli.py#L24-L34

https://github.com/rapidsai/dependency-file-generator/blob/6b0247a23556bf4cc5f5093864cfafd3edb1c363/src/rapids_dependency_file_generator/_cli.py#L117-L118

https://github.com/rapidsai/dependency-file-generator/blob/6b0247a23556bf4cc5f5093864cfafd3edb1c363/src/rapids_dependency_file_generator/_cli.py#L148-L149

import os

p = os.path.dirname(os.path.abspath("README.md"))

type(p)
# <class 'str'>

isinstance(p, os.PathLike)
# False

How I tested this

mypy \
    --ignore-missing-imports \
    --explicit-package-bases \
    ./src
jameslamb commented 3 months ago

Sure, I'll start making them a bit bigger. Was trying to keep them small and focused so they'd be easier to review but I might not have found the right balance there.

After this + #94 are merged I think this typing work could be finished in just 1 or maybe 2 more PRs.

GPUtester commented 3 months ago

:tada: This PR is included in version 1.13.9 :tada:

The release is available on:

Your semantic-release bot :package::rocket: