[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: |
Cleber Rosa |
Subject: |
Re: [PATCH v2 22/38] qapi/source.py: add type hint annotations |
Date: |
Fri, 25 Sep 2020 13:05:24 -0400 |
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 <jsnow@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?
- Cleber.
signature.asc
Description: PGP signature
- [PATCH v2 21/38] qapi/commands.py: enable checking with mypy, (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
[PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic, John Snow, 2020/09/22