-
Notifications
You must be signed in to change notification settings - Fork 14k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Embrace the walrus operator #24127
chore: Embrace the walrus operator #24127
Conversation
Codecov Report
@@ Coverage Diff @@
## master #24127 +/- ##
==========================================
- Coverage 68.30% 68.26% -0.04%
==========================================
Files 1952 1952
Lines 75432 75388 -44
Branches 8191 8202 +11
==========================================
- Hits 51521 51462 -59
- Misses 21807 21819 +12
- Partials 2104 2107 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
26c722a
to
d7e6265
Compare
datasource = form_data.get("datasource", "") | ||
|
||
if "__" in datasource: | ||
# pylint: disable=superfluous-parens |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly we're stuck between a rock and a hard place here. There is a Pylint false positive on line #265 but if we add the disable comment there then Black will reformat the line because its > 88 characters and the formatting then requires the extra (...)
per Pylint's rules.
d7e6265
to
5c89456
Compare
@@ -19,6 +19,10 @@ repos: | |||
rev: 5.12.0 | |||
hooks: | |||
- id: isort | |||
- repo: https://github.com/MarcoGorelli/auto-walrus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should reformat the code before running Mypy, Black, etc.
5c89456
to
ed4e4c8
Compare
Lol, love the auto walrus! Btw walrus was already added in 3.8, so it's been supported on the codebase for quite some time (3.7 was deprecated here in March 2022: #19017) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I browsed through all the changes, and they LGTM. Happy to see this enforced, as I'm also a big fan of the walrus.
SUMMARY
In #23890 (thanks to @sebastianliebscher) we dropped support for Python 3.8 and thus now we are free to embrace the walrus (
:=
) operator. This PR adds a pre-commit hook (which runs beforeblack
) to ensure we never have code of the form,but rather,
Note the pre-commit hook only detects/remedies simple use cases. Personally I've found the walrus operator really handy in list comprehensions (when you need to assign a temporary variable) and logic like,
which becomes,
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
CI.
ADDITIONAL INFORMATION