CLN: GH29547 format with f-strings #34502
Conversation
|
Hello @DanBasson! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-06-20 16:23:55 UTC |
| "cannot do slice indexing on DatetimeIndex with these indexers " | ||
| r"\[{key}\] of type float" | ||
| msg = lambda key: ( | ||
| f"cannot do slice indexing on DatetimeIndex with these indexers " |
There was a problem hiding this comment.
Can you remove the f here and run black to get linting green? I would wait for one of the core devs to give an opinion but usually def is preferred to a named lambda.
There was a problem hiding this comment.
thank you for your response.
if i remove the f in the f-string the code doesn't compile.
may i ask why def is preferred for this? i think that lambda suits better because it's a short statement.
also, i published my solution in the thread but didn't get any response
There was a problem hiding this comment.
Using def vs. lambda is partly a stylistic thing, with the idea being that lambdas are intended as anonymous functions, so naming them somewhat misses the point.
Did you remove the f from all strings? You only need to remove it from the strings for which it has no effect, i.e., when there is nothing to evaluate (e.g., line 87).
|
lgtm. ping on green (the 3.9 biuld is failing but x that) |
|
i don't understand what you are saying (i'm new...). |
it means when the CI are all green, comment. note that the CI is currently having various issues. |
|
the CI are all green. |
|
just to make sure (this error should be patched), can you merge upstream master and then ping on green. |
|
Turns out that was the problem. |
|
thanks @DanBasson |
replace .format() for f-strings in the following: