[DSEW CPR] Pass through python-black

[dshemetov]

Description

Something of a personal preference, as I am usually into black's output. Some main changes:

• consistent white spaces
• consistent use of " versus '
• takes advantage of () to separate long pandas lines, i.e.

# Turn this 
pd.merge(df, max_ts_by_group, on=["publish_date", "geo_id"], how="outer").assign(is_max_group_ts=lambda df: df["timestamp"] == df["max_timestamp"]).drop(["max_timestamp"], axis=1))

# Into this
(
  pd.merge(df, max_ts_by_group, on=["publish_date", "geo_id"], how="outer")
  .assign(is_max_group_ts=lambda df: df["timestamp"] == df["max_timestamp"])
  .drop(["max_timestamp"], axis=1)
)

Merge after the interpolation work.

[dshemetov]

@krivard let's merge this into #1555 only when we're ready to merge that one, so we don't clutter the diffs there.

[dshemetov]

Also let me know if you have any objections to this. It doesn't look great in some parts (like where it has to break many composed functions up and it creates too much white space by creating too many lines with just a single open bracket), but at least it gives us consistency.

[krivard]

Okay this is a really detailed pass but I don't want it to be overwhelming so here's a rough guide:

General case:

• quote normalization is universally 💯
• space-padding around operators, assignment, and parameter specification is universally 💯
• using parens to reduce the need for backslashes and permit the method name to be the first item on a line is almost always 💯
• same-line vs multi-line formatting decisions are too context-dependent to be able to rely on an automated tool 👎

Specifics:

• I marked particularly good reformats with 💯
• I marked particularly bad reformats with "this is worse"
• I marked borderline reformats with "ehhhhhh"
• Anything unmarked is either covered by one of the universal 💯 s above, or was a moderate improvement, neutral, or escaped notice

I'm not 100% sure what this means -- Black doesn't seem to let you turn off particular formatting rules. There does seem to be a mechanism for identifying blocks of code that Black shouldn't touch, but then it wouldn't do the things we do like, and we'd have to maintain an awful lot of those block markers.

[krivard]

this one however is fine, probably because there are fewer levels involved

[dshemetov]

Hm yea, well it's good to learn that same line versus multi-line decisions are really a big weakness of this tool.

Maybe we just keep this a one-off run, keep the parts we like, reject parts we don't, and move on? Like, we're clearly not ready to move to a bot-automated usage, but it's useful if it makes good decisions in like 80% of the cases and we can manually fix the other 20%.

[krivard]

should we refresh this or trash it?

[dshemetov]

I could go either way:

• trash: formatting inconsistency is tolerable, fixing it is low priority, also, it's a bit annoying to have to write #fmt: off around blocks where we don't like the formatting
• refresh: formatting consistency is nice, it's nice to have it automated, so we can focus developer time on more important things

In my personal projects, I use black and just accept its formatting, because I prefer to just not think about it. But as for using it on our team, maybe we should poll our developers' preferences instead.

[nmdefries]

We can close this, CPR has been deprecated.