[PATCH 13/16] qapi/expr.py: Modify check_keys to accept any Iterable

John Snow posted 16 patches 5 years, 4 months ago
There is a newer version of this series
[PATCH 13/16] qapi/expr.py: Modify check_keys to accept any Iterable
Posted by John Snow 5 years, 4 months ago
This is a very minor adjustment.

a + b is list-specific behavior, but we can accept a wider variety of
types in a more pythonic fashion if we avoid that behavior.

Typing it this way allows callers to use things like dict.keys() and
other iterables that are not their own discrete lists.

Including it just as a statement of practice if nothing else: It's nice
to use the least-specific type possible as function input and use the
most-specific type for returns.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/expr.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 4bba09f6e5..2a1f37ca88 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -132,8 +132,8 @@ def check_defn_name_str(name: str, info: QAPISourceInfo, meta: str) -> None:
 def check_keys(value: _JSObject,
                info: QAPISourceInfo,
                source: str,
-               required: List[str],
-               optional: List[str]) -> None:
+               required: Iterable[str] = (),
+               optional: Iterable[str] = ()) -> None:
     """
     Ensures an object has a specific set of keys. [Const]
 
@@ -154,7 +154,7 @@ def pprint(elems: Iterable[str]) -> str:
             "%s misses key%s %s"
             % (source, 's' if len(missing) > 1 else '',
                pprint(missing)))
-    allowed = set(required + optional)
+    allowed = set(required) | set(optional)
     unknown = set(value) - allowed
     if unknown:
         raise QAPISemError(
-- 
2.26.2


Re: [PATCH 13/16] qapi/expr.py: Modify check_keys to accept any Iterable
Posted by Eduardo Habkost 5 years, 4 months ago
On Tue, Sep 22, 2020 at 05:13:10PM -0400, John Snow wrote:
> This is a very minor adjustment.
> 
> a + b is list-specific behavior, but we can accept a wider variety of
> types in a more pythonic fashion if we avoid that behavior.
> 
> Typing it this way allows callers to use things like dict.keys() and
> other iterables that are not their own discrete lists.
> 
> Including it just as a statement of practice if nothing else: It's nice
> to use the least-specific type possible as function input and use the
> most-specific type for returns.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo


Re: [PATCH 13/16] qapi/expr.py: Modify check_keys to accept any Iterable
Posted by Cleber Rosa 5 years, 4 months ago
On Tue, Sep 22, 2020 at 05:13:10PM -0400, John Snow wrote:
> This is a very minor adjustment.
> 
> a + b is list-specific behavior, but we can accept a wider variety of
> types in a more pythonic fashion if we avoid that behavior.
> 
> Typing it this way allows callers to use things like dict.keys() and
> other iterables that are not their own discrete lists.
> 
> Including it just as a statement of practice if nothing else: It's nice
> to use the least-specific type possible as function input and use the
> most-specific type for returns.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Cleber Rosa <crosa@redhat.com>