checks: Flake8 F841 fixes in the wxpython directory part 3

[mshukuno]

Description

Flake8 F841 (local variable assigned to but never used) fixes in the gui/wxpython/gpc and gui/wxpython/gui_core directory.

Motivation and context

Changes require resolving a part of Flake8 warnings that are currently ignored. The issue title and number as follows.

• [Bug] Fix currently ignored Flake8 warnings [Bug] Fix currently ignored Flake8 warnings #1522

How has this been tested?

g.gui.gpc: Briefly tested using GUI and worked fine
gui_core: N/A

Types of changes

• Removed unused variables and lines to fix F841.
• Replaced # pylint: disable=W0612 with # noqa: F841 in gui_core/treeview.py. Please let me know if you want to keep "# pylint". It looks a little odd, but adding # noqa: to the end of code doesn't work. It must be the first line with multiline, like below.

    n111 = tree.AppendNode(  # noqa: F841
        parent=n11, data={"label": "node111", "xxx": "A"}
    )

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as before)

Checklist

• PR title provides summary of the changes and starts with one of the pre-defined prefixes
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.

[petrasovaa]

@wenzeslaus or @echoix not sure about the pylint vs noqa

[echoix]

W0612 is a Pylint equivalent of unused variable.
https://pylint.pycqa.org/en/latest/user_guide/messages/warning/unused-variable.html

[mshukuno]

W0612 is a Pylint equivalent of unused variable. https://pylint.pycqa.org/en/latest/user_guide/messages/warning/unused-variable.html

Flake8 does not recognize the Pylint syntax, and vice versa. I've used the Flake8 syntax to suppress warnings. If you decide to use both linters, I need to add pylint syntax in addition to "noqa:". Since Ruff is an extension of Flake8, using "noqa:" should be no problem.

So, the question is what linter is best for development, Ruff and/or Flake8 or Ruff and/or Flake8 and Pylint?

[echoix]

Flake8 does not recognize the Pylint syntax, and vice versa. I've used the Flake8 syntax to suppress warnings. If you decide to use both linters, I need to add pylint syntax in addition to "noqa:". Since Ruff is an extension of Flake8, using "noqa:" should be no problem.

So, the question is what linter is best for development, Ruff and/or Flake8 or Ruff and/or Flake8 and Pylint?

They serve different purposes. Most notably, Pylint is able to do more complex checks, as it is not a pure static analyzer. It actually imports the files, and can use some information about the types of the variables that can't be known by only checking one file as text. It can also do multi-file analysis. On the other side, ruff reimplements (in rust, not Python), the most important and useful rules from multiple linters+popular plugins, and is real fast. But, it is static only and is (for now) only able to do single-file analysis. Flake8 checks only some rules. Flake8, (like Pylint), has different plugins available. Pylint does not fix issues, flake8 I'm not sure, but at least not with flake8 itself but another tool. Ruff has some rules that have fixes available.

Then, about having both. I saw some two interesting issues but I don't seem to see the conclusion:
pylint-dev/pylint#2470
pylint-dev/pylint#2485

You might want to check a bit more closely either on a search engine or following the links on what was done.

Keep in mind that we are still stuck with a really old Pylint

[wenzeslaus]

@wenzeslaus or @echoix not sure about the pylint vs noqa

At this very point, if ignoring is needed and the choice needs to be made between ignoring with pylint and noqa, we need to go with noqa because this will satisfy the automated checks in the CI (while Pylint is only enforced for a handful of issues). (Given that these can't be combined right now as @echoix points out.)

[mshukuno]

@wenzeslaus or @echoix not sure about the pylint vs noqa

At this very point, if ignoring is needed and the choice needs to be made between ignoring with pylint and noqa, we need to go with noqa because this will satisfy the automated checks in the CI (while Pylint is only enforced for a handful of issues). (Given that these can't be combined right now as @echoix points out.)

It seems okay to keep only Flake8 syntax not Pylint, but does it change if Pylint syntax can also be used with Flake8 syntax? I'm asking this since it is possible.

Pylint 3.2.7
Lines following

1. Position the noqa: command first, followed by the Pylint directive:
   # noqa: 501 pylint: disable=unused-argument

2. Position the directives in any order, but make sure they are both preceded by "# ":
   # pylint: disable=unused-argument # noqa: 501

3. If you also want to have a comment on the same line before the directives you need to add another # after the comment:
   # comment # noqa: 501 pylint: disable=unused-argument

I tested it by adding and removing # pylint: disable=W0612 from # noqa: F841 # pylint: disable=W0612 and running pylint --disable=all --enable=W0612 treeview.py.

It worked as expected for both flake8 and pylint. Let me know if this changes to add Pylint syntaxes.

[echoix]

If you are able to, try it! And if the version of Pylint we use doesn't complain, it's good! (We don't use Pylint v3.x yet)

[wenzeslaus]

My comment was meant for the old Pylint in CI. I'm all for making it work with both, so whatever combo of noqa and pylint works for me.