Leo's PR #4345 revises Leo's source code to correct various flake8 and pyflakes complaints. The flake8 complaints are all valid.
This post discusses new and dubious pyflakes complaints. Imo, these new pyflakes complaints are wrong. Imo, the pyflakes dev should take urgent action to correct this situation.
I plan to open a new pyflakes issue that explains why pyflakes should roll back the offending PR. Here is the first draft. Please review it carefully and suggest improvements.
QQQQQ
Urgent: please roll back pyflakes PR #825.
I am the author of Leo, a large Python app. I use tools such as mypy, flake8, and pylint routinely in my workflow.
But pyflakes is my favorite. It catches 90% or more of my programming blunders. One of Leo's settings causes Leo to run pyflakes to check all newly changed files every time they are written. No other checking tool is fast enough to do such checks.
Alas, I must protest loudly against PR #825. As discussed below, this PR creates great problems for me.
This PR is a violation of your stated design principles:
"Pyflakes makes a simple promise: it will never complain about style, and it will try very, very hard to never emit false positives."
PR #825 does both. It complains about style and creates extremely annoying unwanted false positives. Please roll back this PR immediately. Let me explain:
PR #825 does complain about style, namely flake8 "error" F824. This so-called error is (at best) a redundancy. At worst the error suggests eliminating a use hint for code maintainers.
Within flake8, it doesn't matter whether F824 is an error or a lint. Why? Because it's easy to suppress any flake8 check using #noqa, command-line arguments, or settings files such as setup.cfg.
But within pyflakes, there is (afaik) no way to disable any check. So according to your stated design principles, you must not ever issue a message unless it is (without doubt) a programming blunder. But redundant "global" or "nonlocal" statements are in no way blunders.
The PR's consequences for Leo
Leo's PR #4345 removes the pyflakes messages by commenting out supposedly erroneous "global" statements. But this PR should not be necessary. All code changes might introduce errors, either immediately or (worse) at some much later date.
Furthermore, PR #4345 removes a useful code declaration. If that declaration goes away a later maintainer may introduce an assignment that will then make the "global" declaration necessary. Removing these supposedly "unnecessary" declarations is all pain and no gain.
Alas, without changing Leo's sources, it will be impossible to use pyflakes to perform a batch check of Leo's files without enduring unwanted pyflakes errors.
Summary
PR #825:
- Makes it impossible to use pyflakes on Leo's code base as I would like.
- Creates extremely annoying warnings about valid Python code.
- Sets an unwise precedent of treating flake8 warnings as pyflakes errors.
Please think more carefully about emulating flake8.
According to your stated design principles, pyflakes must not ever issue a message unless it is (without doubt) a programming blunder.
With PR #825 in place, the only way to remove the warnings is to revise the "offending" code. This I am loath to do. Please roll back PR #825 as soon as possible.
Edward
QQQQQ
Yes, this first draft is strident, even after several revisions. I might tone it down further. All comments and suggestions are welcome.
I also plan to close Leo's PR #4345 in favor of a new PR that does not remove the harmless "global" statements. I refuse (or so I think now) to work around these new pyflakes warnings.
Edward
PR #825 does complain about style, namely flake8 "error" F824. This so-called error is (at best) a redundancy. At worst the error suggests eliminating a use hint for code maintainers.
This post discusses new and dubious pyflakes complaints.
I plan to open a new pyflakes issue that explains why pyflakes should roll back the offending PR.
I have just created pyflakes issue #837. All of your comments are welcome.
A pyflakes dev has rejected [ pyflakes issue #837] out of hand and locked it against further discussion.
I don't have many thoughts on this specific issue. However, if I were to encounter a similar problem, in the past, I would have used classic tools like patch, or tools such as https://pypi.org/project/pypatch/, to individually modify the third-party packages needing changes within the Leo environment. Nowadays, I'm more inclined to use fastcore's patch-related functions, as they feel more flexible. If necessary, perhaps consider using this?
I don't have many thoughts on this specific issue. However, if I were to encounter a similar problem, in the past, I would have used classic tools like patch, or tools such as https://pypi.org/project/pypatch/, to individually modify the third-party packages needing changes within the Leo environment. Nowadays, I'm more inclined to use fastcore's patch-related functions, as they feel more flexible. If necessary, perhaps consider using this?