[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 22/38] qapi/source.py: add type hint annotations
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 22/38] qapi/source.py: add type hint annotations |
Date: |
Fri, 25 Sep 2020 14:22:37 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
John Snow <jsnow@redhat.com> writes:
> 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 <jsnow@redhat.com>
[...]
>>> 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!"
>
> When __init__ has some arguments, you only need to type those
> arguments and not the return type. The sentinel is not needed.
>
> __init__ *never* returns anything, so I opted to omit this useless
> annotation whenever it was possible to do so.
Worth capturing in a comment and/or commit message?
- Re: [PATCH v2 16/38] qapi: establish mypy type-checking baseline, (continued)
[PATCH v2 26/38] qapi/gen.py: Enable checking with mypy, John Snow, 2020/09/22
[PATCH v2 25/38] qapi/gen.py: add type hint annotations, John Snow, 2020/09/22