fix(types): resolve type errors and improve type annotations Part 1 #2808
Conversation
| @@ -29,7 +29,7 @@ | |||
| from __future__ import annotations | |||
|
|
|||
| from collections.abc import Awaitable, Callable | |||
| from typing import Union, Type | |||
| from typing import Union | |||
There was a problem hiding this comment.
Please replace the use of Union with |
| @@ -18,9 +18,10 @@ | |||
|
|
|||
| import asyncio | |||
| from collections.abc import Callable | |||
| from typing import Any, Awaitable | |||
There was a problem hiding this comment.
Please import Awaitable from collections.abc instead: https://docs.python.org/3/library/collections.abc.html#collections.abc.Awaitable
| @@ -17,6 +17,7 @@ | |||
| """Action utility module for defining and managing action utilities.""" | |||
|
|
|||
| import inspect | |||
| import typing | |||
| @@ -13,6 +13,7 @@ | |||
| # limitations under the License. | |||
| # | |||
| # SPDX-License-Identifier: Apache-2.0 | |||
| from typing import Any | |||
There was a problem hiding this comment.
nit: blank line before import
please run bin/fmt
| @@ -181,6 +182,6 @@ def test_extract_json(name, input_data, expected_data): | |||
| test_cases_parse_partial_json, | |||
| ids=[tc[0] for tc in test_cases_parse_partial_json], | |||
| ) | |||
| def test_parse_partial_json(name, input_str, expected_data): | |||
| def test_parse_partial_json(name: str, input_str: str, expected_data: dict[str, Any]) -> None: | |||
There was a problem hiding this comment.
nit: docstrings for test methods to reduce linter noise and explain what the test does
py/bin/sanitize_schema_typing.py
Outdated
| @@ -208,7 +210,7 @@ def visit_ClassDef(self, node: ast.ClassDef) -> ast.ClassDef: # noqa: N802 | |||
| # For other classes, just copy the rest of the body | |||
| new_body.extend(node.body[body_start_index:]) | |||
|
|
|||
| node.body = new_body | |||
| node.body = cast( list[ast.stmt], new_body) | |||
There was a problem hiding this comment.
What is the warning that you see here?
There was a problem hiding this comment.
That the node was recognized as a AsT instead of a stmt. Even though the stmt inherit from AsT.
There was a problem hiding this comment.
casts usually are a sign of underlying issues and are us telling the compiler that "we know better." can you please leave this change out of this pr? the rest looks okay
(for future PRs: every cast should be accompanied by your reasoning)
| @@ -83,7 +85,7 @@ | |||
| test_cases_extract_items, | |||
| ids=[tc[0] for tc in test_cases_extract_items], | |||
| ) | |||
| def test_extract_items(name, steps): | |||
| def test_extract_items(name: str, steps: list[dict[str, Any]]) -> None: | |||
There was a problem hiding this comment.
nit: missed docstring here
This is the first part of fixing all the MyPy errors for the genkit projects. It will split the changes on several PR