[libvirt PATCH] scripts: Fix E741 that pycodesyle is pointing out during syntax-check
With newer pycodestyle 2.6.0 (which is part of flake8-3.8.2) reports the following pep violation during syntax-check: ../scripts/check-remote-protocol.py:95:9: E741 ambiguous variable name 'l' for l in err.strip().split("\n") On all the distros we test on, this hasn't occurred yet, but with the future update of flake8 it likely would. The fix is easy, just name the variable appropriately. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- The weird thing is that E741 checking has been in pycodestyle since 2.1.0 [1], we now have 2.5.0 and yet only 2.6.0 is complaining about it [1] https://pycodestyle.pycqa.org/en/latest/developer.html#id8 scripts/check-remote-protocol.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/check-remote-protocol.py b/scripts/check-remote-protocol.py index 25caa19563..cf9e3f84a1 100644 --- a/scripts/check-remote-protocol.py +++ b/scripts/check-remote-protocol.py @@ -92,8 +92,8 @@ if out == "" or pdwtagsproc.returncode != 0: else: print("WARNING: exit code %d, pdwtags appears broken:" % pdwtagsproc.returncode, file=sys.stderr) - for l in err.strip().split("\n"): - print("WARNING: %s" % l, file=sys.stderr) + for line in err.strip().split("\n"): + print("WARNING: %s" % line, file=sys.stderr) print("WARNING: skipping the remote protocol test", file=sys.stderr) sys.exit(0) -- 2.25.4
On 5/25/20 2:56 PM, Erik Skultety wrote:
With newer pycodestyle 2.6.0 (which is part of flake8-3.8.2) reports the following pep violation during syntax-check:
../scripts/check-remote-protocol.py:95:9: E741 ambiguous variable name 'l' for l in err.strip().split("\n")
On all the distros we test on, this hasn't occurred yet, but with the future update of flake8 it likely would. The fix is easy, just name the variable appropriately.
Signed-off-by: Erik Skultety <eskultet@redhat.com> ---
The weird thing is that E741 checking has been in pycodestyle since 2.1.0 [1], we now have 2.5.0 and yet only 2.6.0 is complaining about it [1] https://pycodestyle.pycqa.org/en/latest/developer.html#id8
scripts/check-remote-protocol.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/check-remote-protocol.py b/scripts/check-remote-protocol.py index 25caa19563..cf9e3f84a1 100644 --- a/scripts/check-remote-protocol.py +++ b/scripts/check-remote-protocol.py @@ -92,8 +92,8 @@ if out == "" or pdwtagsproc.returncode != 0: else: print("WARNING: exit code %d, pdwtags appears broken:" % pdwtagsproc.returncode, file=sys.stderr) - for l in err.strip().split("\n"): - print("WARNING: %s" % l, file=sys.stderr) + for line in err.strip().split("\n"): + print("WARNING: %s" % line, file=sys.stderr) print("WARNING: skipping the remote protocol test", file=sys.stderr) sys.exit(0)
Ah, the error is about short variable name, it's about 'l' looking too similar to 'I' (well, if this is somebody's case then I say use better font if you want to code). But in order to shut the checker up: Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
On a Monday in 2020, Michal Privoznik wrote:
On 5/25/20 2:56 PM, Erik Skultety wrote:
With newer pycodestyle 2.6.0 (which is part of flake8-3.8.2) reports the following pep violation during syntax-check:
../scripts/check-remote-protocol.py:95:9: E741 ambiguous variable name 'l' for l in err.strip().split("\n")
On all the distros we test on, this hasn't occurred yet, but with the future update of flake8 it likely would. The fix is easy, just name the variable appropriately.
Signed-off-by: Erik Skultety <eskultet@redhat.com> ---
The weird thing is that E741 checking has been in pycodestyle since 2.1.0 [1], we now have 2.5.0 and yet only 2.6.0 is complaining about it [1] https://pycodestyle.pycqa.org/en/latest/developer.html#id8
scripts/check-remote-protocol.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/check-remote-protocol.py b/scripts/check-remote-protocol.py index 25caa19563..cf9e3f84a1 100644 --- a/scripts/check-remote-protocol.py +++ b/scripts/check-remote-protocol.py @@ -92,8 +92,8 @@ if out == "" or pdwtagsproc.returncode != 0: else: print("WARNING: exit code %d, pdwtags appears broken:" % pdwtagsproc.returncode, file=sys.stderr) - for l in err.strip().split("\n"): - print("WARNING: %s" % l, file=sys.stderr) + for line in err.strip().split("\n"): + print("WARNING: %s" % line, file=sys.stderr) print("WARNING: skipping the remote protocol test", file=sys.stderr) sys.exit(0)
Ah, the error is about short variable name, it's about 'l' looking too similar to 'I' (well, if this is somebody's case then I say use better font if you want to code). But in order to shut the checker up:
Note that we can also shut it up by adding it to our FLAKE8_IGNORE in build-aux/syntax-check.mk, but I don't care either way. Jano
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Michal
On Mon, May 25, 2020 at 06:17:06PM +0200, Ján Tomko wrote:
On a Monday in 2020, Michal Privoznik wrote:
On 5/25/20 2:56 PM, Erik Skultety wrote:
With newer pycodestyle 2.6.0 (which is part of flake8-3.8.2) reports the following pep violation during syntax-check:
../scripts/check-remote-protocol.py:95:9: E741 ambiguous variable name 'l' for l in err.strip().split("\n")
On all the distros we test on, this hasn't occurred yet, but with the future update of flake8 it likely would. The fix is easy, just name the variable appropriately.
Signed-off-by: Erik Skultety <eskultet@redhat.com> ---
The weird thing is that E741 checking has been in pycodestyle since 2.1.0 [1], we now have 2.5.0 and yet only 2.6.0 is complaining about it [1] https://pycodestyle.pycqa.org/en/latest/developer.html#id8
scripts/check-remote-protocol.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/check-remote-protocol.py b/scripts/check-remote-protocol.py index 25caa19563..cf9e3f84a1 100644 --- a/scripts/check-remote-protocol.py +++ b/scripts/check-remote-protocol.py @@ -92,8 +92,8 @@ if out == "" or pdwtagsproc.returncode != 0: else: print("WARNING: exit code %d, pdwtags appears broken:" % pdwtagsproc.returncode, file=sys.stderr) - for l in err.strip().split("\n"): - print("WARNING: %s" % l, file=sys.stderr) + for line in err.strip().split("\n"): + print("WARNING: %s" % line, file=sys.stderr) print("WARNING: skipping the remote protocol test", file=sys.stderr) sys.exit(0)
Ah, the error is about short variable name, it's about 'l' looking too similar to 'I' (well, if this is somebody's case then I say use better font if you want to code). But in order to shut the checker up:
I won't blindly defend all the PEP guidelines, but they do exist for a reason and just like we forbade usage of i,k in other that simple loop use cases, this is a kind of similar on a global scale.
Note that we can also shut it up by adding it to our FLAKE8_IGNORE in build-aux/syntax-check.mk, but I don't care either way.
Of course we can and that was also on the table, but since it's soo trivial to fix and it's also a good practice IMO, I went for a patch to the source instead. Thanks for review. Erik
On 5/26/20 8:48 AM, Erik Skultety wrote:
On Mon, May 25, 2020 at 06:17:06PM +0200, Ján Tomko wrote:
On a Monday in 2020, Michal Privoznik wrote:
On 5/25/20 2:56 PM, Erik Skultety wrote:
With newer pycodestyle 2.6.0 (which is part of flake8-3.8.2) reports the following pep violation during syntax-check:
../scripts/check-remote-protocol.py:95:9: E741 ambiguous variable name 'l' for l in err.strip().split("\n")
On all the distros we test on, this hasn't occurred yet, but with the future update of flake8 it likely would. The fix is easy, just name the variable appropriately.
Signed-off-by: Erik Skultety <eskultet@redhat.com> ---
The weird thing is that E741 checking has been in pycodestyle since 2.1.0 [1], we now have 2.5.0 and yet only 2.6.0 is complaining about it [1] https://pycodestyle.pycqa.org/en/latest/developer.html#id8
scripts/check-remote-protocol.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/check-remote-protocol.py b/scripts/check-remote-protocol.py index 25caa19563..cf9e3f84a1 100644 --- a/scripts/check-remote-protocol.py +++ b/scripts/check-remote-protocol.py @@ -92,8 +92,8 @@ if out == "" or pdwtagsproc.returncode != 0: else: print("WARNING: exit code %d, pdwtags appears broken:" % pdwtagsproc.returncode, file=sys.stderr) - for l in err.strip().split("\n"): - print("WARNING: %s" % l, file=sys.stderr) + for line in err.strip().split("\n"): + print("WARNING: %s" % line, file=sys.stderr) print("WARNING: skipping the remote protocol test", file=sys.stderr) sys.exit(0)
Ah, the error is about short variable name, it's about 'l' looking too similar to 'I' (well, if this is somebody's case then I say use better font if you want to code). But in order to shut the checker up:
I won't blindly defend all the PEP guidelines, but they do exist for a reason and just like we forbade usage of i,k in other that simple loop use cases, this is a kind of similar on a global scale.
I remember us forbiding 'ii', 'jj' and 'kk', not simple 'i', 'j' or 'k'. And we did so because the nested loops are then too messy. My point was that the PEP does not do the same like we do (forbid variables because of their ambiguous name), but because if you use wrong font then they might look the same (unless a variable named 'Iine' is introduced 😈) Anyway, I'm okay with the change, for a C coder whose every Python script is still C with Python syntax, 'line' express it better what I get from 'strip().split("\n")'.
Note that we can also shut it up by adding it to our FLAKE8_IGNORE in build-aux/syntax-check.mk, but I don't care either way.
Of course we can and that was also on the table, but since it's soo trivial to fix and it's also a good practice IMO, I went for a patch to the source instead.
Agreed. Michal
participants (3)
-
Erik Skultety -
Ján Tomko -
Michal Privoznik