On Wed, Feb 15, 2023, 8:36 AM Markus Armbruster <arm...@redhat.com> wrote:
> Markus Armbruster <arm...@redhat.com> writes: > > > John Snow <js...@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 <js...@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 <arm...@redhat.com> > Thanks- everything else look OK enough for now? >
