Re: [PATCH 03/10] qapi: delete un-needed python static analysis configs

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> On Wed, Feb 26, 2025 at 2:28 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > The pylint config is being left in place because the settings differ
>> > enough from the python/ directory settings that we need a chit-chat on
>> > how to merge them O:-)
>> >
>> > Everything else can go.
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>

[...]

>> > diff --git a/scripts/qapi/mypy.ini b/scripts/qapi/mypy.ini
>> > deleted file mode 100644
>> > index 8109470a031..00000000000
>> > --- a/scripts/qapi/mypy.ini
>> > +++ /dev/null
>> > @@ -1,4 +0,0 @@
>> > -[mypy]
>> > -strict = True
>> > -disallow_untyped_calls = False
>> > -python_version = 3.8
>>
>> python/setup.cfg has:
>>
>>    [mypy]
>>    strict = True
>>    python_version = 3.8
>>    warn_unused_configs = True
>>    namespace_packages = True
>>    warn_unused_ignores = False
>>
>> Can you briefly explain the differences?
>>
>
> warn_unused_configs: Catches config values that aren't actually recognized
> or used. Was helpful once upon a time when re-arranging the Python
> directory to behave like a package to ensure that the conf files were
> working correctly.

Could this be culled now?

Hmm, according to mypy(1), strict implies warn-unused-configs.

The question does not block this patch.

> namespace_packages: Needed for the python/ directory structure (nested
> packages under a namespace, "qemu"). Doesn't impact scripts/qapi at all.
> Read up on PEP420 if you are curious. Details in commit message, see below
> if you're still curious.

mypy(1) makes me suspect this is the default.  If that's true across the
versions we care for, this could be culled.

Also does not block this patch.

> warn_unused_ignores: Needed once upon a time for cross-version mypy support
> where some versions would warn in some cases and others would not. Adding
> an ignore would effectively just invert which versions complained. Probably
> still needed, but it's hard to measure.

Harmless enough.

> python_version: Changes mypy behavior regardless of the invoking python
> interpreter to check the file as if it were to be executed by Python 3.8. I
> actually want to remove this value from setup.cfg but haven't yet. I
> removed it from the python-qemu-qmp repo and never added it for qapi.
> Removing it is actually probably correct as it will catch errors specific
> to various python versions we support, but there are some nits to iron out
> in my neck of the woods. This is a case where scripts/qapi/ is stricter
> than python/ :)
> (Not reasonable to solve for this series.)

Also present in the deleted file, so no change.

> lack of disallow_untyped_calls = False: I think this might be a remnant
> from when we gradually typed qapi; it's evidently no longer needed since
> qapi still checks fine without this affordance. The default under strict is
> True.

Fair enough.

> e941c844e444 (John Snow                   2021-05-27 17:17:05 -0400  79)
> [mypy]
> e941c844e444 (John Snow                   2021-05-27 17:17:05 -0400  80)
> strict = True
> ca056f4499c2 (Paolo Bonzini               2023-05-03 12:48:02 +0200  81)
> python_version = 3.8
> e941c844e444 (John Snow                   2021-05-27 17:17:05 -0400  82)
> warn_unused_configs = True
> 0542a4c95767 (John Snow                   2021-05-27 17:17:06 -0400  83)
> namespace_packages = True
> e7874a50ff3f (John Snow                   2022-05-25 20:09:13 -0400  84)
> warn_unused_ignores = False
>
>
>>
>> python/setup.cfg additionally has a bunch of ignore_missing_imports that
>> don't apply here, as far as I can tell.
>>
>
> Right, that's all stuff for fuse and the interactive qmp shell that use
> untyped dependencies.

Good.

Let's mention the differences in the commit message.  Here's my try:

    Since the previous commit, python/setup.cfg applies to scripts/qapi/
    as well.  Configuration files in scripts/qapi/ override
    python/setup.cfg.

    scripts/qapi/.flake8 and scripts/qapi/.isort.cfg actually match
    python/setup.cfg exactly, and can go.

    The differences between scripts/qapi/mypy.ini and python/setup.cfg
    are harmless: [list the differences, explain why they're harmless as
    long as you can keep it brief, and if not, fall back to "trust me"].
    So scripts/qapi/mypy.ini can go, too.

    The pylint config is being left in place because the settings differ
    enough from the python/ directory settings that we need a chit-chat on
    how to merge them O:-)

With something like that
Reviewed-by: Markus Armbruster <armbru@redhat.com>