Skip to content

Stop ignoring Local variable name is assigned to but never used (F841) for tests inflake8 #6661

Open

Description

@hendrikmakait

Problem
When developing, I was surprised that flake8 was not alerting me to unused variables I had left in the code after making some changes. I understand the original reasoning to ignore F841 as laid out in #4888 but I would argue that it helps with refactoring existing test code, making sure that tests are not bloated by legacy code.

Proposal
TL;DR: Boy-scout and flip the switch later.

I do not propose removing F841 from the ignore list and dealing with all the new issues at once. Instead, I would like to remove those over time and flip the switch afterward. The main goal of this issue would be to reach a consensus that we want to remove F841 from the ignore list for tests or highlight any potential issues.

Possible Concerns
One concern brought up by @graingert in an offline discussion was that some unused variables might be used for testing the GC by assigning something to an unused variable to keep it alive. For these cases, I would argue that it is beneficial to add an in-line comment to those unused assignments ignoring F841 and more importantly, explain why this statement is needed. Otherwise, other developers might be tempted to remove the supposedly useless assignment at a later point in time.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions