[Top][All Lists]

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

Re: [PATCH v3 08/16] qapi/expr.py: add type hint annotations

From: John Snow
Subject: Re: [PATCH v3 08/16] qapi/expr.py: add type hint annotations
Date: Thu, 25 Feb 2021 15:54:01 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0

On 2/25/21 8:56 AM, Markus Armbruster wrote:
John Snow <jsnow@redhat.com> writes:

Annotations do not change runtime behavior.
This commit *only* adds annotations.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
  scripts/qapi/expr.py  | 71 ++++++++++++++++++++++++++++---------------
  scripts/qapi/mypy.ini |  5 ---
  2 files changed, 46 insertions(+), 30 deletions(-)

diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index f45d6be1f4c..df6c64950fa 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -15,7 +15,14 @@
  # See the COPYING file in the top-level directory.
import re
-from typing import MutableMapping, Optional, cast
+from typing import (
+    Iterable,
+    List,
+    MutableMapping,
+    Optional,
+    Union,
+    cast,
from .common import c_name
  from .error import QAPISemError
@@ -23,9 +30,10 @@
  from .source import QAPISourceInfo
-# Expressions in their raw form are JSON-like structures with arbitrary forms.
-# Minimally, their top-level form must be a mapping of strings to values.
-Expression = MutableMapping[str, object]
+# Arbitrary form for a JSON-like object.
+_JSObject = MutableMapping[str, object]
+# Expressions in their raw form are (just) JSON-like objects.
+Expression = _JSObject

We solved a similar, slightly more involved typing problem in

Whereas expr.py uses Python dict, list, and scalars to represent the
output of a JSON parser, introspect.py uses them to represent the input
of a quasi-JSON formatter ("quasi-JSON" because it spits out a C
initializer for a C representation of JSON, but that's detail).

introspect.py additionally supports comments and #if conditionals.

This is the solution we're using in introspect.py.  The Annotated[] part
is for comments and conditionals; ignore that.

   # This module constructs a tree data structure that is used to
   # generate the introspection information for QEMU. It is shaped
   # like a JSON value.
   # A complexity over JSON is that our values may or may not be annotated.
   # Un-annotated values may be:
   #     Scalar: str, bool, None.
   #     Non-scalar: List, Dict
   # _value = Union[str, bool, None, Dict[str, JSONValue], List[JSONValue]]
   # With optional annotations, the type of all values is:
   # JSONValue = Union[_Value, Annotated[_Value]]
   # Sadly, mypy does not support recursive types; so the _Stub alias is used to
   # mark the imprecision in the type model where we'd otherwise use JSONValue.
   _Stub = Any
   _Scalar = Union[str, bool, None]
   _NonScalar = Union[Dict[str, _Stub], List[_Stub]]
   _Value = Union[_Scalar, _NonScalar]
   JSONValue = Union[_Value, 'Annotated[_Value]']

introspect.py then adds some more type aliases to convey meaning:

   # These types are based on structures defined in QEMU's schema, so we
   # lack precise types for them here. Python 3.6 does not offer
   # TypedDict constructs, so they are broadly typed here as simple
   # Python Dicts.
   SchemaInfo = Dict[str, object]
   SchemaInfoObject = Dict[str, object]
   SchemaInfoObjectVariant = Dict[str, object]
   SchemaInfoObjectMember = Dict[str, object]
   SchemaInfoCommand = Dict[str, object]

I'm not asking you to factor out common typing.

I'm not even asking you to rework expr.py to maximize similarity.

I am asking you to consider stealing applicable parts from
introspect.py's comments.

_JSObject seems to serve the same purpose as JSONValue.  Correct?

Expression seems to serve a comparable purpose as SchemaInfo.  Correct?


Similar, indeed.

Without annotations:

_Stub = Any
_Scalar = Union[str, bool, None]
_NonScalar = Union[Dict[str, _Stub], List[_Stub]]
_Value = Union[_Scalar, _NonScalar]
JSONValue = _Value

(Or skip the intermediate _Value name. No matter.)

Though expr.py has no use of anything except the Object form itself, because it is inherently a validator and it doesn't actually really require any specific type, necessarily.

So I only really needed the object form, which we never named in introspect.py. We actually avoided naming it.

All I really need is, I think:

_JSONObject = Dict[str, object]

with a comment explaining that object can be any arbitrary JSONValue (within limit for what parser.py is capable of producing), and that the exact form of such will be evaluated by the various check_definition() functions.

Is that suitable, or do you have something else in mind?

reply via email to

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