[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 3/6] qapi: Add minor typing workaround for 3.6
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v4 3/6] qapi: Add minor typing workaround for 3.6 |
Date: |
Wed, 15 Feb 2023 14:36:44 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Markus Armbruster <armbru@redhat.com> writes:
> John Snow <jsnow@redhat.com> writes:
>
>> Pylint under 3.6 does not believe that Collection is subscriptable at
>> runtime. It is, making this a Pylint
>> bug. https://github.com/PyCQA/pylint/issues/2377
>>
>> They closed it as fixed, but that doesn't seem to be true as of Pylint
>> 2.13.9, the latest version you can install under Python 3.6. 2.13.9 was
>> released 2022-05-13, about seven months after the bug was closed.
>>
>> The least-annoying fix here is to just use the more specific type
>> Sequence, only because it seems to work in 3.6.
>>
>> 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 5a1782b57ea..8701351fdfc 100644
>> --- a/scripts/qapi/expr.py
>> +++ b/scripts/qapi/expr.py
>> @@ -33,11 +33,11 @@
>>
>> import re
>> from typing import (
>> - Collection,
>> Dict,
>> Iterable,
>> List,
>> Optional,
>> + Sequence,
>> Union,
>> cast,
>> )
>> @@ -195,8 +195,8 @@ def check_defn_name_str(name: str, info: QAPISourceInfo,
>> meta: str) -> None:
>> def check_keys(value: _JSONObject,
>> info: QAPISourceInfo,
>> source: str,
>> - required: Collection[str],
>> - optional: Collection[str]) -> None:
>> + required: Sequence[str],
>> + optional: Sequence[str]) -> None:
>> """
>> Ensure that a dict has a specific set of keys.
>
> The actual arguments are always List[str]. You actually used that until
> v3 of the patch, and switched to the maximally general Collection[str]
> in v4, with rationale that ended up in commit 538cd41065a:
>
> qapi/expr.py: Modify check_keys to accept any Collection
>
> This is a minor adjustment that lets parameters @required and
> @optional take tuple arguments, in particular (). Later patches will
> make use of that.
>
> No later patch ever did.
>
> I'd prefer maximally stupid List[str], but it's no big deal either way.
Regardless,
Reviewed-by: Markus Armbruster <armbru@redhat.com>
- [PATCH v4 0/6] qapi: static typing conversion, pt5c, John Snow, 2023/02/14
- [PATCH v4 5/6] qapi: remove _JSONObject, John Snow, 2023/02/14
- [PATCH v4 4/6] qapi/parser: add QAPIExpression type, John Snow, 2023/02/14
- Re: [PATCH v4 0/6] qapi: static typing conversion, pt5c, Markus Armbruster, 2023/02/15