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.
Problem
When developing, I was surprised that
flake8was not alerting me to unused variables I had left in the code after making some changes. I understand the original reasoning to ignoreF841as 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
F841from 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 removeF841from 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
F841and 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.