Re: [PATCH v5 25/25] qapi: Dumb down QAPISchema.lookup_entity()

[John Snow]

On Fri, Mar 15, 2024, 11:23 AM Markus Armbruster  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 
> ---
>  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 

>

Re: [PATCH v5 25/25] qapi: Dumb down QAPISchema.lookup_entity()

[Markus Armbruster]

Markus Armbruster  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 

[PATCH v5 25/25] qapi: Dumb down QAPISchema.lookup_entity()

[Markus Armbruster]

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 
---
 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