all: ruff --fix F841 Local variable assigned to but never used. #10994

cclauss · 2023-03-10T06:36:30Z

% ruff rule F841

unused-variable (F841)

Derived from the Pyflakes linter.

Autofix is always available.

What it does

Checks for the presence of unused variables in function scopes.

Why is this bad?

A variable that is defined but not used is likely a mistake, and should
be removed to avoid confusion.

If a variable is intentionally defined-but-not-used, it should be
prefixed with an underscore, or some other value that adheres to the
[dummy-variable-rgx] pattern.

Options

dummy-variable-rgx

Example

def foo():
    x = 1
    y = 2
    return x

Use instead:

def foo():
    x = 1
    return x

github-actions · 2023-03-10T06:43:04Z

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
        rp2:    +0 +0.000% PICO

codecov-commenter · 2023-03-10T07:13:05Z

Codecov Report

Merging #10994 (9789aad) into master (4376c96) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master   #10994   +/-   ##
=======================================
  Coverage   98.50%   98.50%           
=======================================
  Files         155      155           
  Lines       20549    20549           
=======================================
  Hits        20241    20241           
  Misses        308      308

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

stinos · 2023-03-10T07:46:25Z

examples/network/http_server_simplistic.py

@@ -28,7 +28,7 @@ def main():
    while True:
        res = s.accept()
        client_s = res[0]
-        client_addr = res[1]


Does this mean that there is a convention in Python land that names beginning with underscores mean 'unused'? I though it was 'private', which is not the same at all. Wouldn't using # noqa: unused or so be better here? That doesn't need to mangle the name and makes it very explicit and clear that this variable is here only to show what res[1] is. Or even just comment the whole thing like client_addr = res[1]?

Please reread the commit message.

https://dbader.org/blog/meaning-of-underscores-in-python

That doesn't mention unused at all, only private?

Also what do you think about the other suggestions?

Given that this local variable is never used again, _ indicates that it is considered private (to this line alone).

Ignore variables starting with underscore _ PyCQA/pyflakes#393 from 2018.

The # noqa: F841 is possible (# noqa: unused is not) but clunky when reading the code and most contributors will not have memorized ruff's 500 rules.

That doesn't need to mangle the name

Who cares if the name is mangled if it never gets used?

My hope is that when maintainers review this pull request, they spot the logic error and remove the whole line if that makes sense to them. For me, it would just be a SWAG because I do not know this codebase.

It is interesting to note that ruff --select=F841 --fix . is more brutal than am.

client_addr = res[1] --> res[1] # Which is clearly a noop.

To which flake-bugbear would then reply:

B018: Found useless expression. Either assign it to a variable or remove it.

(bugbear's rules are the best!)

and remove the whole line

That would actually be my preference as well

Who cares if the name is mangled if it never gets used?

The reader. The line not being there means less reading.

I would be happy close this PR and you can create a replacement if you would like.
pipx install ruff && ruff --select=F841 --fix . is easy to run.

I'd say just remove this line.

This is an example and is actually a simplified version of http_server_simplistic_commented.py. The latter uses this variable, so if someone wants more details they should look at http_server_simplistic_commented.py instead.

dpgeorge · 2023-05-02T03:10:42Z

examples/network/http_server_simplistic.py

@@ -28,7 +28,7 @@ def main():
    while True:
        res = s.accept()
        client_s = res[0]
-        client_addr = res[1]


I'd say just remove this line.

This is an example and is actually a simplified version of http_server_simplistic_commented.py. The latter uses this variable, so if someone wants more details they should look at http_server_simplistic_commented.py instead.

dpgeorge · 2023-05-02T03:11:07Z

ports/mimxrt/boards/make-pins.py

@@ -226,7 +226,7 @@ def parse_af_file(self, filename, iomux_filename):
            # Extract indexes from header row
            pad_col = header.index("Pad")
            adc_col = header.index("ADC")
-            acmp_col = header.index("ACMP")
+            _acmp_col = header.index("ACMP")


I'd suggest to remove this line.

dpgeorge · 2023-05-02T03:11:22Z

ports/rp2/boards/make-pins.py

@@ -228,7 +228,7 @@ def parse_board_file(self, filename):

                cpu_pin_name = row[1]
                try:
-                    pin_num = parse_pin(cpu_pin_name)
+                    _pin_num = parse_pin(cpu_pin_name)


I'd suggest to just do parse_pin(cpu_pin_name).

dpgeorge · 2023-05-02T03:11:40Z

tools/mpy-tool.py

@@ -1276,7 +1275,7 @@ def read_raw_code(reader, parent_name, qstr_table, obj_table, segments):
                if native_scope_flags & MP_SCOPE_FLAG_VIPERRODATA:
                    rodata_size = reader.read_uint()
                if native_scope_flags & MP_SCOPE_FLAG_VIPERBSS:
-                    bss_size = reader.read_uint()
+                    _bss_size = reader.read_uint()


I'd suggest:

reader.read_uint() # bss size

dpgeorge · 2023-05-02T03:12:07Z

tools/mpy-tool.py

@@ -1285,10 +1284,10 @@ def read_raw_code(reader, parent_name, qstr_table, obj_table, segments):
                        if op == 0xFF:
                            break
                        if op & 1:
-                            addr = reader.read_uint()
+                            _addr = reader.read_uint()


As above, I suggest:

reader.read_uint() # addr

dpgeorge · 2023-05-02T03:12:22Z

tools/mpy-tool.py

                        op >>= 1
                        if op <= 5 and op & 1:
-                            n = reader.read_uint()
+                            _n = reader.read_uint()


As above:

reader.read_uint() # n

dpgeorge · 2023-05-02T03:12:34Z

tools/mpy_ld.py

@@ -471,7 +471,7 @@ def do_relocation_text(env, text_addr, r):
    # Extract relevant info about symbol that's being relocated
    s = r.sym
    s_bind = s.entry["st_info"]["bind"]
-    s_shndx = s.entry["st_shndx"]
+    _s_shndx = s.entry["st_shndx"]


Line can be removed.

This fixes ruff rule F841.

dpgeorge · 2023-05-02T06:38:28Z

Rebased and updated commit message.

stinos reviewed Mar 10, 2023

View reviewed changes

cclauss mentioned this pull request Mar 10, 2023

all: GitHub Action to lint Python code with ruff. #10977

Merged

cclauss requested a review from stinos March 10, 2023 09:20

cclauss force-pushed the ruff_--fix_Local_variable_assigned_to_but_never_used branch from 9789aad to eec36bd Compare March 10, 2023 13:11

cclauss changed the title ~~ruff --fix F841 Local variable assigned to but never used~~ all: ruff --fix F841 Local variable assigned to but never used. Mar 10, 2023

dpgeorge reviewed May 2, 2023

View reviewed changes

cclauss force-pushed the ruff_--fix_Local_variable_assigned_to_but_never_used branch from eec36bd to e7be2bb Compare May 2, 2023 06:08

cclauss requested a review from dpgeorge May 2, 2023 06:08

dpgeorge approved these changes May 2, 2023

View reviewed changes

all: Fix cases of Python variable assigned but never used.

2a1db77

This fixes ruff rule F841.

dpgeorge force-pushed the ruff_--fix_Local_variable_assigned_to_but_never_used branch from e7be2bb to 2a1db77 Compare May 2, 2023 06:38

dpgeorge merged commit 2a1db77 into micropython:master May 2, 2023

cclauss deleted the ruff_--fix_Local_variable_assigned_to_but_never_used branch May 2, 2023 06:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

all: ruff --fix F841 Local variable assigned to but never used. #10994

all: ruff --fix F841 Local variable assigned to but never used. #10994

cclauss commented Mar 10, 2023

github-actions bot commented Mar 10, 2023

codecov-commenter commented Mar 10, 2023 •

edited

Loading

stinos Mar 10, 2023 •

edited

Loading

cclauss Mar 10, 2023

stinos Mar 10, 2023

cclauss Mar 10, 2023 •

edited

Loading

stinos Mar 10, 2023

cclauss Mar 10, 2023 •

edited

Loading

dpgeorge May 2, 2023

dpgeorge May 2, 2023

dpgeorge May 2, 2023

dpgeorge May 2, 2023

dpgeorge May 2, 2023

dpgeorge May 2, 2023

dpgeorge May 2, 2023

dpgeorge May 2, 2023

dpgeorge commented May 2, 2023

all: ruff --fix F841 Local variable assigned to but never used. #10994

all: ruff --fix F841 Local variable assigned to but never used. #10994

Conversation

cclauss commented Mar 10, 2023

unused-variable (F841)

What it does

Why is this bad?

Options

Example

github-actions bot commented Mar 10, 2023

codecov-commenter commented Mar 10, 2023 • edited Loading

Codecov Report

stinos Mar 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cclauss Mar 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cclauss Mar 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpgeorge commented May 2, 2023

codecov-commenter commented Mar 10, 2023 •

edited

Loading

stinos Mar 10, 2023 •

edited

Loading

cclauss Mar 10, 2023 •

edited

Loading

cclauss Mar 10, 2023 •

edited

Loading