CLN: String formatting % -> f-strings #29518
Conversation
pandas/core/arrays/period.py
Outdated
| @@ -600,7 +600,7 @@ def _format_native_types(self, na_rep="NaT", date_format=None, **kwargs): | |||
| if date_format: | |||
| formatter = lambda dt: dt.strftime(date_format) | |||
| else: | |||
| formatter = lambda dt: "%s" % dt | |||
| formatter = lambda dt: f"{dt}" | |||
There was a problem hiding this comment.
would str(dt) be clearer here?
pandas/_version.py
Outdated
| @@ -84,7 +84,7 @@ def run_command(commands, args, cwd=None, verbose=False, hide_stderr=False): | |||
| return None | |||
| else: | |||
| if verbose: | |||
| print("unable to find command, tried %s" % (commands,)) | |||
There was a problem hiding this comment.
i think this file is auto-generated
pandas/core/frame.py
Outdated
| @@ -2187,7 +2187,7 @@ def to_parquet( | |||
| compression=compression, | |||
| index=index, | |||
| partition_cols=partition_cols, | |||
| **kwargs | |||
| **kwargs, | |||
There was a problem hiding this comment.
is this a black version thing?
There was a problem hiding this comment.
Weirdly using 19.3b0 locally seems to add this for me - I've reverted
There was a problem hiding this comment.
Black code check in CI fail without this - i've added this back in
There was a problem hiding this comment.
Personally, I find this quite odd that you have to add a comma at the end.
There was a problem hiding this comment.
Related to this - #29607
|
looks like black complaints |
|
@jbrockmendel - ready to re-review whenever you have a moment |
pandas/core/indexes/base.py
Outdated
| @@ -960,14 +960,13 @@ def __repr__(self): | |||
| data = self._format_data() | |||
| attrs = self._format_attrs() | |||
| space = self._format_space() | |||
|
|
|||
| prepr = (",%s" % space).join("%s=%s" % (k, v) for k, v in attrs) | |||
| prepr = f",{space}".join(f"{k}={v}" for k, v in attrs) | |||
There was a problem hiding this comment.
let's try to minimize what we execute in f-space (better name for this?)
pandas/core/indexes/base.py
Outdated
| @@ -1121,13 +1120,13 @@ def _summary(self, name=None): | |||
| tail = self[-1] | |||
| if hasattr(tail, "format") and not isinstance(tail, str): | |||
| tail = tail.format() | |||
| index_summary = ", %s to %s" % (pprint_thing(head), pprint_thing(tail)) | |||
| index_summary = f", {pprint_thing(head)} to {pprint_thing(tail)}" | |||
There was a problem hiding this comment.
pprint_thing may be unnecessary here now that we're py3-only. can you take a look
| @@ -429,7 +429,7 @@ def __call__(self): | |||
| ).format(estimate=estimate, dmin=dmin, dmax=dmax, arg=self.MAXTICKS * 2) | |||
| ) | |||
|
|
|||
| freq = "%dL" % self._get_interval() | |||
| freq = f"{self._get_interval()}L" | |||
There was a problem hiding this comment.
call get_intterval outside
pandas/core/indexes/datetimelike.py
Outdated
| @@ -496,7 +496,7 @@ def _format_attrs(self): | |||
| if attrib == "freq": | |||
| freq = self.freqstr | |||
| if freq is not None: | |||
| freq = "'%s'" % freq | |||
| freq = f"'{freq}'" | |||
There was a problem hiding this comment.
do the single quotes become unnecessary with a {freq:r} or something like that? This is a pretty common pattern, but we aren't very consistent about when we do or dont use the quotes (i sometimes use ticks like for markdown)
There was a problem hiding this comment.
Assuming you mean {freq!r} here - updated. Agree doesn't seem to be much consistency
|
A handful of comments, general rule about minimizing the code that is executed inside f-space. If its viable to have the black-version-related stuff not in this diff that'll make it a little easier. cc @gfyoung for another look |
|
Looks pretty good! I agree with the comments here. |
|
Updated as per comments and green cc. @jbrockmendel and @gfyoung |
|
thanks @alimcmaster1 |
Follow up from CI: Drop Python 3.5 support #29212
Towards Replace old string formatting syntax with f-strings #29547
passes
black pandaspasses
git diff upstream/master -u -- "*.py" | flake8 --diffOld style % formatting to f-strings