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 have some in "pt5d" that do - but this is the set of patches that do some optional cleanups that you've dropped from earlier sets.
>
> I'd prefer maximally stupid List[str], but it's no big deal either way.
Regardless,
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Thanks- everything else look OK enough for now?