Fix incorrect type annotations and type inference #445
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
added 6 commits March 30, 2023 15:24
This function takes as input a pair of *sequences* of 2-tuples (not a pair of 2-tuples) and returns a list of 2-tuples.
When using mypy to check the package, it will attempt to infer types that are not specified. Currently, mypy is able to understand that constructing a dict from a Sequence[Tuple[X, Y]] yields a Dict[X, Y], but if we use lists instead of tuples, mypy doesn't understand and seems to think the result is a Dict[X, X]. This is probably a bug in mypy, but in any case, using tuples here rather than lists is more idiomatic and doesn't affect the behavior of the code.
mypy (and likely other static analysis tools) will complain if an object attribute is used without being defined either in the class or the class's __init__ method. Declare the attribute here so that mypy knows it exists.
This function requires as input a Sequence of signal names, not merely a Collection. (There's no particular reason this function *couldn't* be written to accept any Collection or even any Iterable, but at present it requires the argument to be something that implements both __len__ and __getitem__.)
mypy (and likely other static analysis tools) will complain if an object attribute is used without being defined either in the class or the class's __init__ method. The MultiHeaderMixin class is not public and not meant to be instantiated at all, but refers to attributes that are defined by the MultiRecord class which inherits from it. Declare the types of these attributes here so that mypy knows they exist.
This variable is used in a strange way; initially it contains string values extracted from the record line, then later these string values are replaced with appropriately-typed values for each field. However, there's no real way to make this type-safe without changing the API of the function. Annotate the variable's type so that mypy will not treat it as a Dict[str, Optional[str]].
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Some of the type annotations that have been added in recent versions are just plain wrong. Also, some code constructs are too complicated for mypy to be able to infer types.
With the
mainversion:With this version:
Note that
--ignore-missing-importsis needed because some of the required packages are not annotated or incompletely annotated.