qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 08/19] qapi/schema: add static typing and assertions to looku


From: John Snow
Subject: Re: [PATCH 08/19] qapi/schema: add static typing and assertions to lookup_type()
Date: Tue, 21 Nov 2023 11:46:37 -0500



On Tue, Nov 21, 2023, 9:21 AM Markus Armbruster <armbru@redhat.com> wrote:
John Snow <jsnow@redhat.com> writes:

> This function is a bit hard to type as-is; mypy needs some assertions to
> assist with the type narrowing.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qapi/schema.py | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index a1094283828..3308f334872 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -968,8 +968,12 @@ def lookup_entity(self, name, typ=None):
>              return None
>          return ent

> -    def lookup_type(self, name):
> -        return self.lookup_entity(name, QAPISchemaType)
> +    def lookup_type(self, name: str) -> Optional[QAPISchemaType]:

Any particular reason not to delay the type hints until PATCH 16?

I forget. In some cases I did things a little differently so that the type checking would pass for each patch in the series, which sometimes required some concessions.

Is this one of those cases? Uh, I forget.

If it isn't, its almost certainly the case that I just figured I'd type this one function in one place instead of splitting it apart into two patches.

I can try to shift the typing later and see what happens if you prefer it that way.


> +        typ = self.lookup_entity(name, QAPISchemaType)
> +        if typ is None:
> +            return None
> +        assert isinstance(typ, QAPISchemaType)
> +        return typ

Would

           typ = self.lookup_entity(name, QAPISchemaType)
           assert isinstance(typ, Optional[QAPISchemaType])
           return typ

work?

I don't *think* so, Optional isn't a runtime construct. We can combine it into "assert x is None or isinstance(x, foo)" though - I believe that's used elsewhere in the qapi generator.



>      def resolve_type(self, name, info, what):
>          typ = self.lookup_type(name)


reply via email to

[Prev in Thread] Current Thread [Next in Thread]