| Message ID | 20240315152301.3621858-26-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: statically type schema.py | expand |
Markus Armbruster <armbru@redhat.com> writes: > QAPISchema.lookup_entity() takes an optional type argument, a subtype > of QAPISchemaDefinition, and returns that type or None. Callers can > use this to save themselves an isinstance() test. > > The only remaining user of this convenience feature is .lookup_type(). > But we don't actually save anything anymore there: we still the we still need the > isinstance() to help mypy over the hump. > > Drop the .lookup_entity() argument, and adjust .lookup_type(). > > Signed-off-by: Markus Armbruster <armbru@redhat.com>
On Fri, Mar 15, 2024, 11:23 AM Markus Armbruster <armbru@redhat.com> wrote: > QAPISchema.lookup_entity() takes an optional type argument, a subtype > of QAPISchemaDefinition, and returns that type or None. Callers can > use this to save themselves an isinstance() test. > > The only remaining user of this convenience feature is .lookup_type(). > But we don't actually save anything anymore there: we still the > isinstance() to help mypy over the hump. > > Drop the .lookup_entity() argument, and adjust .lookup_type(). > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > scripts/qapi/schema.py | 18 ++++++------------ > 1 file changed, 6 insertions(+), 12 deletions(-) > > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > index a6180f93c6..5924947fc3 100644 > --- a/scripts/qapi/schema.py > +++ b/scripts/qapi/schema.py > @@ -1157,20 +1157,14 @@ def _def_definition(self, defn: > QAPISchemaDefinition) -> None: > defn.info, "%s is already defined" % > other_defn.describe()) > self._entity_dict[defn.name] = defn > > - def lookup_entity( > - self, > - name: str, > - typ: Optional[type] = None, > - ) -> Optional[QAPISchemaDefinition]: > - ent = self._entity_dict.get(name) > - if typ and not isinstance(ent, typ): > - return None > - return ent > + def lookup_entity(self,name: str) -> Optional[QAPISchemaEntity]: > + return self._entity_dict.get(name) > > def lookup_type(self, name: str) -> Optional[QAPISchemaType]: > - typ = self.lookup_entity(name, QAPISchemaType) > - assert typ is None or isinstance(typ, QAPISchemaType) > - return typ > + typ = self.lookup_entity(name) > + if isinstance(typ, QAPISchemaType): > + return typ > + return None > > def resolve_type( > self, > -- > 2.44.0 > Sure, dealer's choice. with your commit message fixup: Reviewed-by: John Snow <jsnow@redhat.com> >
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index a6180f93c6..5924947fc3 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -1157,20 +1157,14 @@ def _def_definition(self, defn: QAPISchemaDefinition) -> None: defn.info, "%s is already defined" % other_defn.describe()) self._entity_dict[defn.name] = defn - def lookup_entity( - self, - name: str, - typ: Optional[type] = None, - ) -> Optional[QAPISchemaDefinition]: - ent = self._entity_dict.get(name) - if typ and not isinstance(ent, typ): - return None - return ent + def lookup_entity(self,name: str) -> Optional[QAPISchemaEntity]: + return self._entity_dict.get(name) def lookup_type(self, name: str) -> Optional[QAPISchemaType]: - typ = self.lookup_entity(name, QAPISchemaType) - assert typ is None or isinstance(typ, QAPISchemaType) - return typ + typ = self.lookup_entity(name) + if isinstance(typ, QAPISchemaType): + return typ + return None def resolve_type( self,
QAPISchema.lookup_entity() takes an optional type argument, a subtype of QAPISchemaDefinition, and returns that type or None. Callers can use this to save themselves an isinstance() test. The only remaining user of this convenience feature is .lookup_type(). But we don't actually save anything anymore there: we still the isinstance() to help mypy over the hump. Drop the .lookup_entity() argument, and adjust .lookup_type(). Signed-off-by: Markus Armbruster <armbru@redhat.com> --- scripts/qapi/schema.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)