On 31.03.20 19:39, Kevin Wolf wrote:
> Am 31.03.2020 um 19:23 hat John Snow geschrieben:
>>
>>
>> On 3/31/20 6:21 AM, Max Reitz wrote:
>>> On 31.03.20 02:00, John Snow wrote:
>>>> Minor cleanup for HMP functions; helps with line length and consolidates
>>>> HMP helpers through one implementation function.
>>>>
>>>> Although we are adding a universal toggle to turn QMP logging on or off,
>>>> many existing callers to hmp functions don't expect that output to be
>>>> logged, which causes quite a few changes in the test output.
>>>>
>>>> For now, offer a use_log parameter.
>>>>
>>>>
>>>> Typing notes:
>>>>
>>>> QMPResponse is just an alias for Dict[str, Any]. It holds no special
>>>> meanings and it is not a formal subtype of Dict[str, Any]. It is best
>>>> thought of as a lexical synonym.
>>>>
>>>> We may well wish to add stricter subtypes in the future for certain
>>>> shapes of data that are not formalized as Python objects, at which point
>>>> we can simply retire the alias and allow mypy to more strictly check
>>>> usages of the name.
>>>>
>>>> Signed-off-by: John Snow <js...@redhat.com>
>>>> ---
>>>> tests/qemu-iotests/iotests.py | 35 ++++++++++++++++++++++-------------
>>>> 1 file changed, 22 insertions(+), 13 deletions(-)
>>>
>>> Reviewed-by: Max Reitz <mre...@redhat.com>
>>>
>>>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>>>> index b08bcb87e1..dfc753c319 100644
>>>> --- a/tests/qemu-iotests/iotests.py
>>>> +++ b/tests/qemu-iotests/iotests.py
>>>> @@ -37,6 +37,10 @@
>>>>
>>>> assert sys.version_info >= (3, 6)
>>>>
>>>> +# Type Aliases
>>>> +QMPResponse = Dict[str, Any]
>>>> +
>>>> +
>>>> faulthandler.enable()
>>>>
>>>> # This will not work if arguments contain spaces but is necessary if we
>>>> @@ -540,25 +544,30 @@ def add_incoming(self, addr):
>>>> self._args.append(addr)
>>>> return self
>>>>
>>>> - def pause_drive(self, drive, event=None):
>>>> - '''Pause drive r/w operations'''
>>>> + def hmp(self, command_line: str, use_log: bool = False) ->
>>>> QMPResponse:
>>>> + cmd = 'human-monitor-command'
>>>> + kwargs = {'command-line': command_line}
>>>> + if use_log:
>>>> + return self.qmp_log(cmd, **kwargs)
>>>> + else:
>>>> + return self.qmp(cmd, **kwargs)
>>>
>>> Hm. I suppose I should take this chance to understand something about
>>> mypy. QEMUMachine.qmp() isn’t typed, so mypy can’t check that this
>>> really returns QMPResponse. Is there some flag to make it? Like
>>> --actually-check-types?
>>>
>>
>> One of --strict's implied options, I'm not sure which. Otherwise, mypy
>> is geared towards a 'gradual typing' discipline.
>>
>> In truth, I'm a little thankful for that because it helps avoid yak
>> shaving marathons.Sure. I was just looking into the different options. I was interested in whether I could come up with a mode that leaves wholly untyped code alone, but warns for code that mixes it. Or something. >> It does mean that sometimes the annotations don't "do anything" yet, >> apart from offering hints and documentation in e.g. pycharm. Which does >> mean that sometimes they can be completely wrong... >> >> The more we add, the more we'll catch problems. >> >> Once this series is dusted I'll try to tackle more conversions for >> iotests, qmp, etc. I've got a few WIP patches to tackle conversions for >> tests/qemu-iotests/*.py but I am trying to shepherd this one in first >> before I go bananas. Sure, sure. >>> (--strict seems, well, overly strict? Like not allowing generics, I >>> don’t see why. Or I suppose for the time being we want to allow untyped >>> definitions, as long as they don’t break type assertions such as it kind >>> of does here...?) >>> >> >> "disallow-any-generics" means disallowing `Any` generics, not >> disallowing generics ... in general. (I think? I've been using mypy in >> strict mode for a personal project a lot lately and I use generics in a >> few places, it seems OK.) > > --disallow-any-generics > disallow usage of generic types that do not specify explicit type > parameters > > So it will complain if you say just List, and you need to be explicit if > you really want List[Any]. Which I think is a reasonable thing to > require. OK. So it’s “disallow ‘any’ generics”, not “disallow any ‘generic’s”. Not easy to parse. (Yes, yes, I should’ve actually read the man page...) Good to know that mypy and me actually do seem to loosely agree on what a generic is. :) Max
signature.asc
Description: OpenPGP digital signature
