Skip to content

Fix incorrect type annotations and type inference #445

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

Merged
merged 6 commits into from
Mar 30, 2023
Merged

Conversation

bemoody
Copy link
Collaborator

@bemoody bemoody commented Mar 30, 2023

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 main version:

$ mypy --ignore-missing-imports wfdb
wfdb/io/util.py:116: error: Incompatible return value type (got "List[Tuple[Any, Any]]", expected "Tuple[int, int]")
wfdb/io/util.py:117: error: Value of type "int" is not indexable
wfdb/io/util.py:120: error: Value of type "int" is not indexable
wfdb/io/download.py:31: error: "Config" has no attribute "db_index_url"
wfdb/io/download.py:105: error: "Config" has no attribute "db_index_url"
wfdb/io/_header.py:879: error: "MultiHeaderMixin" has no attribute "segments"
wfdb/io/_header.py:892: error: "MultiHeaderMixin" has no attribute "n_seg"
wfdb/io/_header.py:893: error: "MultiHeaderMixin" has no attribute "seg_len"
wfdb/io/_header.py:894: error: "MultiHeaderMixin" has no attribute "segments"
wfdb/io/_header.py:953: error: Value of type "Collection[str]" is not indexable
wfdb/io/_header.py:956: error: Value of type "Collection[str]" is not indexable
wfdb/io/_header.py:957: error: Incompatible types in assignment (expression has type "Tuple[int, int]", variable has type "List[Tuple[int, int]]")
wfdb/io/_header.py:958: error: Argument 1 to "overlapping_ranges" has incompatible type "List[Tuple[int, int]]"; expected "Tuple[int, int]"
wfdb/io/_header.py:958: error: Argument 2 to "overlapping_ranges" has incompatible type "List[Tuple[int, int]]"; expected "Tuple[int, int]"
wfdb/io/_header.py:1040: error: Incompatible types in assignment (expression has type "int", target has type "str")
wfdb/io/_header.py:1042: error: Incompatible types in assignment (expression has type "float", target has type "str")
wfdb/io/_header.py:1048: error: Incompatible types in assignment (expression has type "float", target has type "str")
wfdb/io/_header.py:1050: error: Incompatible types in assignment (expression has type "time", target has type "str")
wfdb/io/_header.py:1054: error: Incompatible types in assignment (expression has type "date", target has type "str")
wfdb/io/_header.py:1060: error: Incompatible types in assignment (expression has type "datetime", target has type "str")
wfdb/io/_header.py:1061: error: Argument 1 to "combine" of "datetime" has incompatible type "str"; expected "date"
wfdb/io/_header.py:1061: error: Argument 2 to "combine" of "datetime" has incompatible type "str"; expected "time"
wfdb/io/record.py:1663: error: Argument 1 to "dict" has incompatible type "List[List[Any]]"; expected "Iterable[Tuple[<nothing>, <nothing>]]"
Found 23 errors in 4 files (checked 29 source files)

With this version:

$ mypy --ignore-missing-imports wfdb
Success: no issues found in 29 source files

Note that --ignore-missing-imports is needed because some of the required packages are not annotated or incompletely annotated.

Benjamin Moody 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]].
Copy link
Member

@tompollard tompollard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@tompollard tompollard merged commit 4d4233f into main Mar 30, 2023
@tompollard tompollard deleted the type-fixes branch March 30, 2023 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants