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>
---
scripts/qapi/.flake8 | 3 ---
scripts/qapi/.isort.cfg | 7 -------
scripts/qapi/mypy.ini | 4 ----
3 files changed, 14 deletions(-)
delete mode 100644 scripts/qapi/.flake8
delete mode 100644 scripts/qapi/.isort.cfg
delete mode 100644 scripts/qapi/mypy.ini
diff --git a/scripts/qapi/.flake8 b/scripts/qapi/.flake8
deleted file mode 100644
index a873ff67309..00000000000
--- a/scripts/qapi/.flake8
+++ /dev/null
@@ -1,3 +0,0 @@
-[flake8]
-# Prefer pylint's bare-except checks to flake8's
-extend-ignore = E722
diff --git a/scripts/qapi/.isort.cfg b/scripts/qapi/.isort.cfg
deleted file mode 100644
index 643caa1fbd6..00000000000
--- a/scripts/qapi/.isort.cfg
+++ /dev/null
@@ -1,7 +0,0 @@
-[settings]
-force_grid_wrap=4
-force_sort_within_sections=True
-include_trailing_comma=True
-line_length=72
-lines_after_imports=2
-multi_line_output=3
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
--
2.48.1John 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>
> ---
> scripts/qapi/.flake8 | 3 ---
> scripts/qapi/.isort.cfg | 7 -------
> scripts/qapi/mypy.ini | 4 ----
> 3 files changed, 14 deletions(-)
> delete mode 100644 scripts/qapi/.flake8
> delete mode 100644 scripts/qapi/.isort.cfg
> delete mode 100644 scripts/qapi/mypy.ini
>
> diff --git a/scripts/qapi/.flake8 b/scripts/qapi/.flake8
> deleted file mode 100644
> index a873ff67309..00000000000
> --- a/scripts/qapi/.flake8
> +++ /dev/null
> @@ -1,3 +0,0 @@
> -[flake8]
> -# Prefer pylint's bare-except checks to flake8's
> -extend-ignore = E722
python/setup.cfg has:
[flake8]
# Prefer pylint's bare-except checks to flake8's
extend-ignore = E722
exclude = __pycache__,
Good.
> diff --git a/scripts/qapi/.isort.cfg b/scripts/qapi/.isort.cfg
> deleted file mode 100644
> index 643caa1fbd6..00000000000
> --- a/scripts/qapi/.isort.cfg
> +++ /dev/null
> @@ -1,7 +0,0 @@
> -[settings]
> -force_grid_wrap=4
> -force_sort_within_sections=True
> -include_trailing_comma=True
> -line_length=72
> -lines_after_imports=2
> -multi_line_output=3
python/setup.cfg has:
[isort]
force_grid_wrap=4
force_sort_within_sections=True
include_trailing_comma=True
line_length=72
lines_after_imports=2
multi_line_output=3
Good.
> 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
Also a bunch of [mypy-FOO] sections that don't apply here.
You explained the differences in review of a prior iteration. Recap:
} 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.
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>
On Tue, Mar 25, 2025 at 4:05 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> > > --- > > scripts/qapi/.flake8 | 3 --- > > scripts/qapi/.isort.cfg | 7 ------- > > scripts/qapi/mypy.ini | 4 ---- > > 3 files changed, 14 deletions(-) > > delete mode 100644 scripts/qapi/.flake8 > > delete mode 100644 scripts/qapi/.isort.cfg > > delete mode 100644 scripts/qapi/mypy.ini > > > > diff --git a/scripts/qapi/.flake8 b/scripts/qapi/.flake8 > > deleted file mode 100644 > > index a873ff67309..00000000000 > > --- a/scripts/qapi/.flake8 > > +++ /dev/null > > @@ -1,3 +0,0 @@ > > -[flake8] > > -# Prefer pylint's bare-except checks to flake8's > > -extend-ignore = E722 > > python/setup.cfg has: > > [flake8] > # Prefer pylint's bare-except checks to flake8's > extend-ignore = E722 > exclude = __pycache__, > > Good. > > > diff --git a/scripts/qapi/.isort.cfg b/scripts/qapi/.isort.cfg > > deleted file mode 100644 > > index 643caa1fbd6..00000000000 > > --- a/scripts/qapi/.isort.cfg > > +++ /dev/null > > @@ -1,7 +0,0 @@ > > -[settings] > > -force_grid_wrap=4 > > -force_sort_within_sections=True > > -include_trailing_comma=True > > -line_length=72 > > -lines_after_imports=2 > > -multi_line_output=3 > > python/setup.cfg has: > > [isort] > force_grid_wrap=4 > force_sort_within_sections=True > include_trailing_comma=True > line_length=72 > lines_after_imports=2 > multi_line_output=3 > > Good. > > > 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 > > Also a bunch of [mypy-FOO] sections that don't apply here. > > You explained the differences in review of a prior iteration. Recap: > > } 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? > Maybe! > > Hmm, according to mypy(1), strict implies warn-unused-configs. > > The question does not block this patch. > Send me a patch to drop it O:-) > > } 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. > It definitely wasn't once upon a time. It may still not be true on the oldest mypy we currently support. We don't have a clear policy for what versions of python libraries we need to support - this is a muddy, gray area. So far I just try to avoid breaking support on older versions needlessly, but I don't have an upgrade policy. If we want to integrate this directly into make check, we'll likely need to formalize this policy. > > } 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. > > 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> > okey-dokey, let me integrate this feedback and I'll re-send, but I'm going to wait until we hash everything else out - you had some questions on other bits in this series.
John Snow <jsnow@redhat.com> writes:
> On Tue, Mar 25, 2025 at 4:05 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
>>
>> Also a bunch of [mypy-FOO] sections that don't apply here.
>>
>> You explained the differences in review of a prior iteration. Recap:
>>
>> } 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?
>>
>
> Maybe!
>
>
>>
>> Hmm, according to mypy(1), strict implies warn-unused-configs.
>>
>> The question does not block this patch.
>>
>
> Send me a patch to drop it O:-)
Done:
[PATCH] python: Drop redundant warn_unused_configs = True
Message-ID: <20250326071203.238931-1-armbru@redhat.com>
>> } 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.
>>
>
> It definitely wasn't once upon a time. It may still not be true on the
> oldest mypy we currently support. We don't have a clear policy for what
> versions of python libraries we need to support - this is a muddy, gray
> area. So far I just try to avoid breaking support on older versions
> needlessly, but I don't have an upgrade policy.
So what is the oldest mypy we currently support? Unknown, best effort
to fix any breakage we see? Wouldn't quite match my dictionary's
definition of "support"...
> If we want to integrate this directly into make check, we'll likely need to
> formalize this policy.
My gut feeling: supporting old mypy isn't worth much (if any) trouble.
>> } 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.
>>
>> 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>
>>
>
> okey-dokey, let me integrate this feedback and I'll re-send, but I'm going
> to wait until we hash everything else out - you had some questions on other
> bits in this series.
Thanks!
On Wed, Mar 26, 2025 at 3:18 AM Markus Armbruster <armbru@redhat.com> wrote: > John Snow <jsnow@redhat.com> writes: > > > On Tue, Mar 25, 2025 at 4:05 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 > >> > >> Also a bunch of [mypy-FOO] sections that don't apply here. > >> > >> You explained the differences in review of a prior iteration. Recap: > >> > >> } 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? > >> > > > > Maybe! > > > > > >> > >> Hmm, according to mypy(1), strict implies warn-unused-configs. > >> > >> The question does not block this patch. > >> > > > > Send me a patch to drop it O:-) > > Done: > > [PATCH] python: Drop redundant warn_unused_configs = True > Message-ID: <20250326071203.238931-1-armbru@redhat.com> > yay :) > > >> } 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. > >> > > > > It definitely wasn't once upon a time. It may still not be true on the > > oldest mypy we currently support. We don't have a clear policy for what > > versions of python libraries we need to support - this is a muddy, gray > > area. So far I just try to avoid breaking support on older versions > > needlessly, but I don't have an upgrade policy. > > So what is the oldest mypy we currently support? Unknown, best effort > to fix any breakage we see? Wouldn't quite match my dictionary's > definition of "support"... > from python/setup.cfg; mypy >= 1.4.0 from python/tests/minreqs.txt; mypy==1.4.0 This version is used for the minreqs test so I can be assured that we haven't broken anything for the old pythons and the old type checkers. I guarantee this version *will* work. I also guarantee the latest version will work. I don't necessarily guarantee everything in between, but I make my best effort. > > > If we want to integrate this directly into make check, we'll likely need > to > > formalize this policy. > > My gut feeling: supporting old mypy isn't worth much (if any) trouble. > Very extremely likely true! If people are fine with "you need PyPI and and internet connection to run linter tests *at all*" we could move to the latest version(s) (as supported per-python interpreter, at least) and be done with it. As I recall, some folks (Kevin Wolf?) had some requirements that they be able to run iotests and so forth outside of configure, which makes this kind of thing harder. The tests grew out of ad-hoc scripts that had to work with whatever you happened to have installed, though, and so it still supports a wide range of linter versions for that reason. If I had my way, I'd stipulate we only ever use a specific version that I pin quite close to the current bleeding edge and then just periodically update the pin as needed for new features or bugfixes or compatibility for shifting python interpreter support windows. If you want to discuss this and tackle it next release, I'm open to it, I've just lost a *lot* of appetite for making new package version requirement edicts in and around the build/doc/test system. > > >> } 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. > >> > >> 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> > >> > > > > okey-dokey, let me integrate this feedback and I'll re-send, but I'm > going > > to wait until we hash everything else out - you had some questions on > other > > bits in this series. > > Thanks! > >
© 2016 - 2025 Red Hat, Inc.