Skip to content

/ pandas

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CLN remove unnecessary trailing commas to get ready for new version of black #35925

Open
MarcoGorelli opened this issue Aug 27, 2020 · 5 comments
Open

CLN remove unnecessary trailing commas to get ready for new version of black #35925

MarcoGorelli opened this issue Aug 27, 2020 · 5 comments
Labels
Clean good first issue
Milestone
Contributions Wel...

Comments

@MarcoGorelli
Copy link
Member

@MarcoGorelli MarcoGorelli commented Aug 27, 2020 •
edited

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:

  • pandas/_testing.py
  • pandas/core/aggregation.py
  • pandas/core/algorithms.py
  • pandas/core/arrays/_mixins.py
  • pandas/core/arrays/categorical.py
  • pandas/core/arrays/integer.py
  • pandas/core/arrays/masked.py
  • pandas/core/arrays/numpy_.py
  • pandas/core/arrays/period.py
  • pandas/core/construction.py
  • pandas/core/generic.py
  • pandas/core/groupby/generic.py
  • pandas/core/groupby/groupby.py
  • pandas/core/groupby/ops.py
  • pandas/core/indexes/datetimelike.py
  • pandas/core/indexes/interval.py
  • pandas/core/indexes/numeric.py
  • pandas/core/indexes/range.py
  • pandas/core/internals/blocks.py
  • 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
  • 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
  • pandas/io/parquet.py
  • pandas/io/parsers.py
  • pandas/io/pytables.py
  • 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
  • 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
  • pandas/tests/frame/test_analytics.py
  • pandas/tests/frame/test_constructors.py
  • pandas/tests/frame/test_reshape.py
  • pandas/tests/generic/test_finalize.py
  • pandas/tests/generic/test_to_xarray.py
  • pandas/tests/groupby/aggregate/test_numba.py
  • pandas/tests/groupby/test_apply.py
  • pandas/tests/groupby/test_categorical.py
  • pandas/tests/groupby/test_groupby.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
  • pandas/tests/indexes/datetimes/test_datetime.py
  • pandas/tests/indexes/datetimes/test_timezones.py
  • pandas/tests/indexes/multi/test_constructors.py
  • pandas/tests/indexes/multi/test_isin.py
  • pandas/tests/indexes/test_base.py
  • pandas/tests/indexes/timedeltas/test_scalar_compat.py
  • pandas/tests/indexes/timedeltas/test_searchsorted.py
  • pandas/tests/indexing/common.py
  • pandas/tests/indexing/test_callable.py
  • pandas/tests/indexing/test_check_indexer.py
  • pandas/tests/indexing/test_coercion.py
  • pandas/tests/indexing/test_floats.py
  • pandas/tests/indexing/test_iloc.py
  • pandas/tests/indexing/test_indexing.py
  • pandas/tests/indexing/test_loc.py
  • pandas/tests/internals/test_internals.py
  • pandas/tests/io/formats/test_css.py
  • pandas/tests/io/formats/test_info.py
  • pandas/tests/io/json/test_compression.py
  • pandas/tests/io/json/test_pandas.py
  • pandas/tests/io/parser/test_c_parser_only.py
  • pandas/tests/io/parser/test_parse_dates.py
  • pandas/tests/io/parser/test_usecols.py
  • 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
  • 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
  • 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
@simonjayhawkins simonjayhawkins added this to the Contributions Welcome milestone Aug 27, 2020
This was referenced Aug 27, 2020
Closed
Closed
Merged
@MarcoGorelli
Copy link
Member Author

@MarcoGorelli MarcoGorelli commented Aug 27, 2020 •
edited

See #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)
@jorisvandenbossche
Copy link
Member

@jorisvandenbossche jorisvandenbossche commented Aug 27, 2020

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)
This was referenced Aug 28, 2020
Merged
Merged
@jpribyl
Copy link
Contributor

@jpribyl jpribyl commented Aug 28, 2020 •
edited

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: #35956

@MarcoGorelli if it looks good I can open another with some more later today.
@jpribyl jpribyl mentioned this issue Aug 28, 2020
Merged
@MarcoGorelli
Copy link
Member Author

@MarcoGorelli MarcoGorelli commented Aug 28, 2020

@jpribyl thanks! Yes, feel free to do so, any help here would be appreciated!
@jpribyl jpribyl mentioned this issue Aug 28, 2020
Merged
@jpribyl
Copy link
Contributor

@jpribyl jpribyl commented Aug 28, 2020

#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
This was referenced Aug 29, 2020
Closed
Closed
JonathanShrek added a commit to JonathanShrek/pandas that referenced this issue Aug 30, 2020
@JonathanShrek
Comma cleanup for pandas-dev#35925
d097609
JonathanShrek added a commit to JonathanShrek/pandas that referenced this issue Aug 30, 2020
@JonathanShrek
Comma cleanup for pandas-dev#35925
c34ed0e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Clean good first issue
Projects
None yet
Milestone
Contributions Wel...
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
@jorisvandenbossche @simonjayhawkins @jpribyl @MarcoGorelli
You can’t perform that action at this time.