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

chore(refactor): prefer typing.Any to 'object' for type hints #93

Merged
merged 1 commit into from
May 14, 2024

Conversation

jameslamb
Copy link
Member

Contributes to #87.

Proposes changing type hints using object to typing.Any.

This fixes 10 errors from mypy.

full list (click me)
src/rapids_dependency_file_generator/_config.py:171: error: Argument 1 to "_parse_outputs" has incompatible type "object"; expected "Union[str, list[str]]"  [arg-type]
src/rapids_dependency_file_generator/_config.py:173: error: No overload variant of "list" matches argument type "object"  [call-overload]
src/rapids_dependency_file_generator/_config.py:173: note: Possible overload variants:
src/rapids_dependency_file_generator/_config.py:173: note:     def [_T] list(self) -> list[_T]
src/rapids_dependency_file_generator/_config.py:173: note:     def [_T] list(self, Iterable[_T], /) -> list[_T]
src/rapids_dependency_file_generator/_config.py:174: error: "object" has no attribute "items"  [attr-defined]
src/rapids_dependency_file_generator/_config.py:175: error: Argument 1 to "Path" has incompatible type "object"; expected "Union[str, PathLike[str]]"  [arg-type]
src/rapids_dependency_file_generator/_config.py:176: error: Argument 1 to "Path" has incompatible type "object"; expected "Union[str, PathLike[str]]"  [arg-type]
src/rapids_dependency_file_generator/_config.py:177: error: Argument 1 to "Path" has incompatible type "object"; expected "Union[str, PathLike[str]]"  [arg-type]
src/rapids_dependency_file_generator/_config.py:195: error: "object" has no attribute "__iter__"; maybe "__dir__" or "__str__"? (not iterable)  [attr-defined]
src/rapids_dependency_file_generator/_config.py:208: error: "object" has no attribute "__iter__"; maybe "__dir__" or "__str__"? (not iterable)  [attr-defined]
src/rapids_dependency_file_generator/_config.py:244: error: "object" has no attribute "items"  [attr-defined]
src/rapids_dependency_file_generator/_config.py:246: error: "object" has no attribute "items"  [attr-defined]

Notes for Reviewers

What's the difference between object and typing.Any?

I'll let the typing docs explain:

This behavior allows Any to be used as an escape hatch when you need to mix dynamically and statically typed code.

Contrast the behavior of Any with the behavior of object. Similar to Any, every type is a subtype of object.
However, unlike Any ... object is not a subtype of every other type.

(docs link)

This is why with a type hint like object you get errors like this:

_config.py:244: error: "object" has no attribute "items"

typing.Any tells the type checker "just ignore this object from this point on in the stack".

Shouldn't we use a more specific type than typing.Any?

I don't think so.

These hints with typing.Any are only in the parts of the codebase taking in mostly-unstructured data read in with yaml.load(). Those functions (parse_config() and the things it calls) are taking on the responsibility of imposing structure and are the only place working with the raw dictionary representation of the YAML.

I think it's fairly low-risk to use typing.Any in that situation and simpler than, say, creating a type alias with a union of all the types that yaml.load() could produce in its dictionary representation of the YAML.

pyyaml's own type stubs use typing.Any for the return value of safe_load(): https://github.com/python/typeshed/blob/a9d7e861f7a46ae7acd56569326adef302e10f29/stubs/PyYAML/yaml/__init__.pyi#L38.

@jameslamb jameslamb added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels May 14, 2024
@jameslamb jameslamb merged commit 6c52975 into rapidsai:main May 14, 2024
4 checks passed
@jameslamb jameslamb deleted the mypy/any-types branch May 14, 2024 18:42
@GPUtester
Copy link

🎉 This PR is included in version 1.13.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves an existing functionality non-breaking Introduces a non-breaking change released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants