script, test: python linter flake8 E275 fixup, update dependencies

[jonatack]

It is helpful to be able to run the python linter locally to review PRs and check local diffs and work. Fix the errors raised by ./test/lint/lint-python.py when run locally with flake8 5.0.4, which enforces rule E275 more strictly than previous versions, and update our python linter CI dependencies.

[stickies-v]

Approach ACK - this has been on my to-do list too. I prefer fixing the issue as opposed to dropping rules.

[stickies-v]

ACK 395519b

Nice to have lint-python.py run without all those warnings. Non-controversial change imo, could have put a space instead of removing the parenthesis but this is more pythonic so I'm in favour. We do have instances of assert (...) left where the parentheses could be removed but it's orthogonal and not worth the change on its own, imo.

[theStack]

Concept ACK

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	kristapsk
Concept ACK	theStack
Stale ACK	w0xlt, willcl-ark, stickies-v, brunoerg

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #26356 (test: Use consistent assert functions by NorrinRadd)
• #26325 (rpc: Return accurate results for scanblocks by aureleoules)
• #26222 (Introduce field element and group element classes to test_framework/key.py by sipa)
• #24005 (test: add python implementation of Elligator swift by stratospher)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

[brunoerg]

Concept ACK

[w0xlt]

ACK 395519b

[maflcko]

CI is red

[maflcko]

Also, not sure if we should enforce this whitespace, given that no other whitespace is enforced.

[jonatack]

Rebased and updated with latest fixups (CI was green on the last push with 2 ACKs here, so I suppose it was restarted to verify with the latest merges).

[jonatack]

Also, not sure if we should enforce this

As mentioned in the pull description, another option would be to drop the E275 rule.

[maflcko]

CI is still red.

To me it seems odd to enforce this whitespace but no other. If you want to enforce whitespace, it might be best to discuss in a separate pull, and then for all whitespace, unless there is a reason to do it for only this one.

[jonatack]

Rebased and CI is green. The intent is only to have our python linter pass on CI and locally with the current version; E275 seems to be part of current flake8 (https://www.flake8rules.com/rules/E275.html) unless we opt out of it. Like @stickies-v, I mildly prefer fixing the issue with more pythonic code rather than dropping rules.

[fanquake]

~0

Is there a particular reason this change is broken into commits per-directory? Seems like an odd way to break it up.
Could squash the 4, and use a commit message more explicit than "fixups" given there's only a single change here?

[fanquake]

See also 634cb38 from #26468, that uses a scripted-diff, which seems like the best way to do this.

[jonatack]

See also 634cb38 from #26468, that uses a scripted-diff, which seems like the best way to do this.

I find that version less pythonic and the other comments are nits. It would be nice to fix this and this has been open for a while.

[fanquake]

and the other comments are nits.

I don't think suggesting the use of a commit message that clearly explains a change, rather than multiple vauge messages, is a nit.

The message from 634cb38: "Fix E275 errors in lint-python.py with flake8 5.0.4", explains what's being done, and why. This is much more useful when compared to "X: python linter X fixups" repeated 4 times.

[willcl-ark]

ACK fc3ca6f

Happy to re-review if you do change up the commit messages.

I do prefer the result of this PR over how my scripted diff turned it out (sorry again @jonatack for opening that dupe without searching first!)

[stickies-v]

re-ACK fc3ca6f

However, I think fanquake's suggestion to make the commit messages more helpful is on point, so I would support doing that first. I hereby commit to quickly re-ACK once that's done (assuming no other big changes).

[brunoerg]

ACK fc3ca6f

Tested locally the versions modified in 04_install and reviewed the changes in the code, most are similar. I agree on squashing and making the message more helpful, happy to re-ack if you address it!

[jonatack]

Rebased and addressed feedback. Opened #26801 as an alternative. I mildly prefer this version for its more pythonic code.

[kristapsk]

ACK 1e5e87c

[kristapsk]

I mildly prefer this version for its more pythonic code.

I also prefer this over #26801.