On 9/25/20 1:05 PM, Cleber Rosa wrote:
On Wed, Sep 23, 2020 at 07:55:50PM -0400, John Snow wrote:On 9/23/20 6:36 PM, Cleber Rosa wrote:On Tue, Sep 22, 2020 at 05:00:45PM -0400, John Snow wrote:Annotations do not change runtime behavior. This commit *only* adds annotations.Signed-off-by: John Snow <js...@redhat.com> --- scripts/qapi/mypy.ini | 5 ----- scripts/qapi/source.py | 31 ++++++++++++++++++------------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/scripts/qapi/mypy.ini b/scripts/qapi/mypy.ini index 9da1dccef4..43c8bd1973 100644 --- a/scripts/qapi/mypy.ini +++ b/scripts/qapi/mypy.ini @@ -39,11 +39,6 @@ disallow_untyped_defs = False disallow_incomplete_defs = False check_untyped_defs = False -[mypy-qapi.source] -disallow_untyped_defs = False -disallow_incomplete_defs = False -check_untyped_defs = False -This is what I meant in my comment in the previous patch. It looks like a mix of commit grannurality styles. Not a blocker though.Yep. Just how the chips fell. Some files were just very quick to cleanup and I didn't have to refactor them much when I split things out, so the enablements got rolled in. I will, once reviews are in (and there is a commitment to merge), try to squash things where it seems appropriate.[mypy-qapi.types] disallow_untyped_defs = False disallow_incomplete_defs = False diff --git a/scripts/qapi/source.py b/scripts/qapi/source.py index e97b9a8e15..1cc6a5b82d 100644 --- a/scripts/qapi/source.py +++ b/scripts/qapi/source.py @@ -11,37 +11,42 @@ import copy import sys +from typing import List, Optional, TypeVar class QAPISchemaPragma: - def __init__(self): + def __init__(self) -> None:I don't follow the reason for typing this...# Are documentation comments required? self.doc_required = False # Whitelist of commands allowed to return a non-dictionary - self.returns_whitelist = [] + self.returns_whitelist: List[str] = [] # Whitelist of entities allowed to violate case conventions - self.name_case_whitelist = [] + self.name_case_whitelist: List[str] = [] class QAPISourceInfo: - def __init__(self, fname, line, parent): + T = TypeVar('T', bound='QAPISourceInfo') + + def __init__(self: T, fname: str, line: int, parent: Optional[T]):And not this... to tune my review approach, should I assume that this series intends to add complete type hints or not?This is a mypy quirk you've discovered that I've simply forgotten about. When __init__ has *no* arguments, you need to annotate its return to explain to mypy that you have fully typed that method. It's a sentinel that says "Please type check this class!"Ouch. Is this a permanent quirk or a known bug that will eventually be addressed?
Permanent, it is a feature.mypy intentionally supports gradual typing as a paradigm: it allows you to intermix "typed" and "untyped" functions.
```
def __init__(self):
pass
```
Happens to pass as both untyped and fully typed. In order to distinguish
it in this one case, you must add the return annotation as a declaration
of intent.
However, when using '--strict' mode, you are declaring your intent to mypy that everything MUST be strictly typed, so perhaps in this case it would be possible to omit the annotation for __init__.
So maybe someday this will change; but given how uncommon it is to write no-argument init methods, I am hardly bothered by it. Mypy will remind you if you forget.
- Cleber.
