On Tue, Nov 21, 2023, 9:21 AM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@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 <js...@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) > >
