format replaced with f-strings #29701
Conversation
| @@ -1751,7 +1749,7 @@ def nth(self, n: Union[int, List[int]], dropna: Optional[str] = None) -> DataFra | |||
| raise ValueError( | |||
| "For a DataFrame groupby, dropna must be " | |||
| "either None, 'any' or 'all', " | |||
| "(was passed {dropna}).".format(dropna=dropna) | |||
| f"(was passed {dropna})." | |||
There was a problem hiding this comment.
@datapythonista do we have a preferred usage for cases where this line needs the "f" and the preceeding does not? I could imagine we might want to put the "f" on all of them anyway
There was a problem hiding this comment.
I think this is correct - should just keep it on lines where needed
There was a problem hiding this comment.
Agree with @WillAyd, my preference is to just have the f were it's needed. Even in strings splitted in different lines.
There was a problem hiding this comment.
As described in pep-498-concatenating-strings, regular strings are concatenated at compile time, and f-strings are concatenated at run time.
I think what we should not concatenate regular strings with f-strings because this could lead to unexpected results.
| @@ -212,13 +208,13 @@ def groups(self): | |||
|
|
|||
| def __repr__(self) -> str: | |||
| attrs_list = ( | |||
| "{name}={val!r}".format(name=attr_name, val=getattr(self, attr_name)) | |||
| f"{attr_name}={getattr(self, attr_name)!r}" | |||
There was a problem hiding this comment.
can you do this in two steps so as to not have the getattr inside the fstring
There was a problem hiding this comment.
@jbrockmendel Not really sure how to do this in two steps since attr_name is generated from a list comprehension. Do I just drop list comprehension usage in that case?
There was a problem hiding this comment.
attr_tups = ((name, getatttr(self, name)) for name in self._attributes)
attr_tups = (x for x in attr_tups if x[1] is not None)
attrs_list = (f"{attr_name}={attr_val!r}" of attr_name, attr_val in attr_tups)
pandas/core/groupby/grouper.py
Outdated
| "len(data)\nresult: {grper}".format( | ||
| grper=pprint_thing(self.grouper) | ||
| ) | ||
| f"len(data)\nresult: {pprint_thing(self.grouper)}" |
There was a problem hiding this comment.
pls take the pprint_thing outside of fspace
|
@datapythonista @WillAyd Great! Thanks for the approval. One more thing, I'm not sure what went wrong with the CI tests, but it doesn't seem related to these changes. What should I do? Cheers |
The affected builds are being flaky for unrelated reasons. We're trying to fix them, and will probably ask you to rebase when that is done |
No problem! |
|
alright can you rebase and we'll see if the CI goes through |
lucassa3
force-pushed
the
replace-to-f-strings
branch
from
1706e90 to
dd51a3a
Compare
Rebased |
|
thanks @lucassa3 |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffref #29547