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
[WIP][MAINT] implement walrus operator? #4202
base: main
Are you sure you want to change the base?
Conversation
👋 @Remi-Gau Thanks for creating a PR! Until this PR is ready for review, you can include the [WIP] tag in its title, or leave it as a github draft. Please make sure it is compliant with our contributing guidelines. In particular, be sure it checks the boxes listed below.
For new features:
For bug fixes:
We will review it as quick as possible, feel free to ping us with questions if needed. |
walrus operator is a python 3.8 feature so just adding a pre-commit hook to enforce it given that we have dropped python 3.7 Curious to see how others feel about it. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #4202 +/- ##
==========================================
- Coverage 92.09% 91.88% -0.22%
==========================================
Files 144 144
Lines 16366 16348 -18
Branches 3426 3164 -262
==========================================
- Hits 15073 15021 -52
- Misses 754 785 +31
- Partials 539 542 +3 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I like it but some of its effects that may not be clear makes me hesitant about enforcing it. No strong opinion though. Here's this blog post about it: https://martinheinz.dev/blog/79 |
No strong feeling about this. We should follow what the majority of the community considers best practices. |
this is defo not urgent so we can wait to get more feedback |
weak -1 for me. even though it's been around for a few releases I don't see the walrus operator being used that often; I wouldn't be surprised if some contributors don't know what it is; the updated code in the diff is not really better than the old one so I don't see a reason for the change. As 3.7 support is dropped I don't mind new code using that operator when the contributor wants to use it, but I don't see the logic behind systematically enforcing its use wherever it could possibly be applied |
more generally I would really like to avoid going overboard with the precommit and linting stuff and only enforce coding style rules that actually make the codebase easier to maintain or contribute to (including for new contributors) |
Changes proposed in this pull request: