Pandas: CLN remove unnecessary trailing commas to get ready for new version of black

Created on 27 Aug 2020 · 20Comments · Source: pandas-dev/pandas

The new version of black is consistent in how it handles the magic trailing commas. So if we upgrade black and apply it, lots of files will be changed. However, the diff needn't be so large if we remove unnecessary trailing commas before upgrading.

E.g. in pandas/core/aggregation.py there is

def reconstruct_func(
    func: Optional[AggFuncType], **kwargs,
) -> Tuple[
    bool, Optional[AggFuncType], Optional[List[str]], Optional[List[int]],
]:

which has an unnecessary trailing comma.

The new version of black would transform this as

def reconstruct_func(
    func: Optional[AggFuncType],
    **kwargs,
) -> Tuple[bool, Optional[AggFuncType], Optional[List[str]], Optional[List[int]],]:

However, if we instead remove the trailing comma and write it as

def reconstruct_func(
    func: Optional[AggFuncType], **kwargs
) -> Tuple[bool, Optional[AggFuncType], Optional[List[str]], Optional[List[int]]]:

then both the current and the new versions of black will be OK with it.

So, PRs to remove some unnecessary trailing commas would be welcome - perhaps keep each PR limited to 5-10 files changed.

Files that (may) need changing are:

[ ] /asv_bench/benchmarks/arithmetic.py
[ ] /pandas/core/array_algos/replace.py
[x] /doc/make.py
[ ] /doc/source/conf.py
[ ] /pandas/core/aggregation.py
[ ] /pandas/core/algorithms.py
[ ] /pandas/_vendored/typing_extensions.py
[ ] /pandas/core/util/numba_.py
[ ] /pandas/core/sorting.py
[ ] /pandas/io/formats/latex.py
[ ] /pandas/core/series.py
[ ] /pandas/io/formats/format.py
[ ] /pandas/core/frame.py
[ ] /pandas/tests/arrays/sparse/test_array.py
[ ] /pandas/tests/frame/test_analytics.py
[x] /pandas/tests/indexes/base_class/test_indexing.py
[x] /pandas/tests/indexes/test_common.py
[x] /pandas/tests/indexes/test_numeric.py
[ ] /pandas/tests/io/test_gcs.py
[ ] /pandas/tests/io/test_parquet.py
[ ] /pandas/tests/scalar/timestamp/test_constructors.py
[ ] /pandas/tests/series/test_operators.py
[ ] /scripts/tests/test_validate_docstrings.py

EDIT

updated list of files which need changing

Clean good first issue

Source

MarcoGorelli

Most helpful comment

https://github.com/pandas-dev/pandas/pull/35959

with several more files:

pandas/core/series.py
pandas/core/window/ewm.py
pandas/core/window/indexers.py
pandas/core/window/numba_.py
pandas/core/window/rolling.py
pandas/io/formats/css.py
pandas/io/formats/format.py
pandas/io/orc.py

jpribyl on 28 Aug 2020

🎉3

All 20 comments

See https://github.com/pandas-dev/pandas/pull/35930, #35949, #35950 and for examples of how to open a PR for this.

No need to ask for permission to work on this, just choose a few files to work on and open a PR (optionally, you can comment here indicating that you're working on them)

MarcoGorelli on 27 Aug 2020

👍1

I think the easiest way to find them is actually to let black (using the latest version) reformat the file(s), and then check in the diff for which ones the trailing comma can be removed (and then run black again)

jorisvandenbossche on 27 Aug 2020

👍3

I am looking at these right now

[ ] pandas/core/internals/concat.py
[ ] pandas/core/internals/managers.py
[ ] pandas/core/internals/ops.py
[ ] pandas/core/nanops.py
[ ] pandas/core/ops/docstrings.py
[ ] pandas/core/reshape/concat.py
[ ] pandas/core/reshape/pivot.py
[ ] pandas/core/reshape/reshape.py

edit: https://github.com/pandas-dev/pandas/pull/35956

@MarcoGorelli if it looks good I can open another with some more later today.

jpribyl on 28 Aug 2020

🎉1

@jpribyl thanks! Yes, feel free to do so, any help here would be appreciated!

MarcoGorelli on 28 Aug 2020

🎉1

https://github.com/pandas-dev/pandas/pull/35959

with several more files:

pandas/core/series.py
pandas/core/window/ewm.py
pandas/core/window/indexers.py
pandas/core/window/numba_.py
pandas/core/window/rolling.py
pandas/io/formats/css.py
pandas/io/formats/format.py
pandas/io/orc.py

jpribyl on 28 Aug 2020

🎉3

working on these right now:

pandas/tests/test_multilevel.py
pandas/tests/test_nanops.py
pandas/tests/window/moments/test_moments_consistency_rolling.py
pandas/tests/window/moments/test_moments_ewm.py
pandas/tests/window/moments/test_moments_rolling.py
pandas/tests/window/test_base_indexer.py
pandas/tests/window/test_pairwise.py
pandas/tests/window/test_rolling.py
pandas/tseries/frequencies.py
pandas/util/_test_decorators.py

35996

metehankutlu on 30 Aug 2020

👍1

I'm searching using this regex: ,\n? *\)|,\n? *\] What do you guys think? Is it the correct way?

Edit: ,\n? *(\)|\]|\})

metehankutlu on 30 Aug 2020

I'm searching using this regex: ,\n? *\)|,\n? *\] What do you guys think? Is it the correct way?

Edit: ,\n? *(\)|\]|\})

I would just upgrade black, run it on the chosen files, see what's changed, remove trailing commas, and run black again

MarcoGorelli on 30 Aug 2020

👍2

I would just upgrade black, run it on the chosen files, see what's changed, remove trailing commas, and run black again

@MarcoGorelli when I am running black again, it changes file back with trailing commas

rajanshoo25 on 1 Sep 2020

I would just upgrade black, run it on the chosen files, see what's changed, remove trailing commas, and run black again

@MarcoGorelli when I am running black again, it changes file back with trailing commas

That's fine. Two things may happen when running the new version of black:

the command is long, and so it splits it into multiple lines, each with a trailing comma;
the command is short, and so it puts it all on one line.

It's the second case in which we want to remove the trailing commas, so that both the current and the new version of black would reformat that line in the same way.

From a quick glance it looks like you've done it correctly anyway, will review later today

MarcoGorelli on 1 Sep 2020

👍1

Working on these files:

pandas/io/sas/sas_xport.py
pandas/io/stata.py
pandas/tests/arithmetic/test_interval.py
pandas/tests/arithmetic/test_numeric.py
pandas/tests/arrays/boolean/test_logical.py

tiagohonorato on 1 Sep 2020

Now Working on

pandas/tests/scalar/test_na_scalar.py
- pandas/tests/scalar/timestamp/test_arithmetic.py
- pandas/tests/scalar/timestamp/test_constructors.py
- pandas/tests/series/methods/test_argsort.py
- pandas/tests/series/methods/test_convert_dtypes.py
- pandas/tests/series/methods/test_drop_duplicates.py
- pandas/tests/series/methods/test_interpolate.py
- pandas/tests/series/methods/test_unstack.py
- pandas/tests/series/test_cumulative.py
- pandas/tests/test_algos.py

sarthakchaudhary13 on 1 Sep 2020

Working on:

pandas/tests/groupby/aggregate/test_numba.py
pandas/tests/groupby/test_apply.py
pandas/tests/groupby/test_categorical.py
pandas/tests/groupby/test_groupby_dropna.py
pandas/tests/groupby/test_groupby_subclass.py
pandas/tests/groupby/test_size.py
pandas/tests/groupby/test_timegrouper.py
pandas/tests/groupby/transform/test_numba.py

xcz011 on 2 Sep 2020

Working on these files

pandas/tests/io/test_sql.py
pandas/tests/io/test_stata.py
pandas/tests/plotting/test_frame.py
pandas/tests/resample/test_datetime_index.py
pandas/tests/reshape/merge/test_merge_index_as_string.py
pandas/tests/reshape/test_crosstab.py
pandas/tests/reshape/test_get_dummies.py

sarthakchaudhary13 on 2 Sep 2020

hello, I'm new to this community.. Now I'm working on these files :

pandas/tests/arrays/categorical/test_replace.py
pandas/tests/arrays/sparse/test_array.py
pandas/tests/arrays/test_array.py
pandas/tests/arrays/test_timedeltas.py
pandas/tests/base/test_conversion.py
pandas/tests/dtypes/test_missing.py
pandas/tests/extension/base/methods.py
pandas/tests/extension/test_sparse.py
pandas/tests/frame/indexing/test_setitem.py

satrio-hw on 7 Sep 2020

Working on:

pandas/tests/io/pytables/test_timezones.py
pandas/tests/io/test_feather.py
pandas/tests/io/test_s3.py
pandas/tests/io/test_sql.py
pandas/tests/io/test_stata.py
pandas/tests/plotting/test_frame.py
pandas/tests/resample/test_datetime_index.py
pandas/tests/reshape/merge/test_merge_index_as_string.py
pandas/tests/reshape/test_crosstab.py
pandas/tests/reshape/test_get_dummies.py

tiagohonorato on 8 Sep 2020

Working On:
pandas/tests/series/methods/test_interpolate.py
pandas/tests/series/methods/test_unstack.py
pandas/tests/series/test_cumulative.py
pandas/tests/test_algos.py