Type hints for tests and examples #1859
Conversation
miguelgrinberg
force-pushed
the
types-tests-and-examples
branch
3 times, most recently
from
e63d0ce to
bfbf67a
Compare
miguelgrinberg
changed the title
| self, | ||
| bucket: bool, | ||
| name: str, | ||
| agg_type: Union[Dict[str, Any], Agg[_R], str], |
There was a problem hiding this comment.
mypy was perfectly happy with my naive agg_type: str type, but pyright flagged an issue in one of the examples that pointed at this as the problem.
| tags = Keyword(multi=True) | ||
| content = Text() | ||
| if TYPE_CHECKING: | ||
| _id: int |
There was a problem hiding this comment.
This is a trick to get the dataclass support to accept these extra arguments in the constructor.
There was a problem hiding this comment.
Since this is an example, should we explain the trick?
There was a problem hiding this comment.
Yes, I should add a comment here, thanks.
| # these are still supported and should be kept functional in spite | ||
| # of not having appropriate type hints. For that reason the comment | ||
| # below disables many mypy checks that fails as a result of this. | ||
| # mypy: disable-error-code="assignment, index, arg-type, call-arg, operator, comparison-overlap, attr-defined" |
There was a problem hiding this comment.
There are a couple of tests files that have a lot of errors because of documents that are defined using old syntax withtout the type hints. I felt it was best to silence those errors and keep some of those older syntax examples.
There was a problem hiding this comment.
Should we also keep an untyped example without all the typing "clutter"?
miguelgrinberg
force-pushed
the
types-tests-and-examples
branch
2 times, most recently
from
6f44786 to
7341cc9
Compare
noxfile.py
Outdated
| @@ -38,8 +36,6 @@ | |||
| "tests/test_query.py", | |||
| "tests/test_utils.py", | |||
| "tests/test_wrappers.py", | |||
| "examples/vectors.py", | |||
| "examples/async/vectors.py", | |||
| ) | |||
There was a problem hiding this comment.
Yes, I forgot.
| @@ -50,6 +52,7 @@ async def test_alias_migration(async_write_client): | |||
| # _matches work which means we get BlogPost instance | |||
| bp = (await BlogPost.search().execute())[0] | |||
| assert isinstance(bp, BlogPost) | |||
| print("**", bp.published) | |||
miguelgrinberg
force-pushed
the
types-tests-and-examples
branch
from
7341cc9 to
eaff847
Compare
* Type hints for tests and examples * add pyright check for examples only * add pyright check for examples only * simplified examples * review feedback (cherry picked from commit 663d6f4)
* Type hints for tests and examples * add pyright check for examples only * add pyright check for examples only * simplified examples * review feedback (cherry picked from commit 663d6f4) Co-authored-by: Miguel Grinberg <[email protected]>
Type hints for all remaining tests and examples.
There are some notes embedded in the code, but the main changes are:
Documentinstances I have silenced a number of mypy errors that come as a result of using the old way of creating document fields without type hints. This is because I want to make sure these continue to be supported and tested.AttrList. In all current usages we are doingAttrList[Any]andAttrDict[Any], but this might change in the future.Work that remains wrt types:
Any. It would be useful to create specific types for all the fields.Listvs.Sequenceand dicts withDictandMappingorMutableMappingthat I will try to clean up at some point.