Skip to content

Blacken the code base #27076

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

Merged
merged 3 commits into from
Jul 4, 2019
Merged

Conversation

jorisvandenbossche
Copy link
Member

So we can see what it looks like

@jreback jreback added the Code Style Code style, linting, code_checks label Jun 27, 2019
@jreback jreback added the Blocker Blocking issue or pull request for an upcoming release label Jun 27, 2019
@jreback jreback added this to the 0.25.0 milestone Jun 27, 2019
@jorisvandenbossche
Copy link
Member Author

@TomAugspurger a question we are having here at the sprint, is that if you add it as a pre-commit hook, does it run black on the full code base (as that takes a while for pandas) ?

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Jun 27, 2019 via email

@jorisvandenbossche
Copy link
Member Author

The test with 79 limit added around 20,000 lines

@TomAugspurger
Copy link
Contributor

@simonjayhawkins is the black default of 88 too wide?

@simonjayhawkins
Copy link
Member

@simonjayhawkins is the black default of 88 too wide?

I'm happy to go with the black default and see how it goes, if nobody else is objecting strongly.

The test with 79 limit added around 20,000 lines

maybe too much nested code!

@jreback
Copy link
Contributor

jreback commented Jul 3, 2019

@jorisvandenbossche should prob merge this right after the RC is cut

@WillAyd
Copy link
Member

WillAyd commented Jul 3, 2019 via email

@jreback
Copy link
Contributor

jreback commented Jul 4, 2019

@jorisvandenbossche sorry just merged the last 2 PRs

@jorisvandenbossche
Copy link
Member Author

No problem, the main work here was updating the fixup commits and checking they are still fine for flake8 / isort. Once we actually put it in, I can just cherry-pick them on top of the latest blackened one.

@jorisvandenbossche
Copy link
Member Author

Do we want to do this now? (after the RC is fine for me as well) @TomAugspurger are you still planning to tag tonight?

@jreback
Copy link
Contributor

jreback commented Jul 4, 2019

cool, I would say fix this up and let's push it in!

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Jul 4, 2019

Yep, fine with doing this now.

@jorisvandenbossche
Copy link
Member Author

Do we also want to blacken the asv benchmarks ? (currently I only did the actual pandas package)

Re-applying it now

@jreback
Copy link
Contributor

jreback commented Jul 4, 2019

i think blacken everything as we flake everything

@TomAugspurger
Copy link
Contributor

@jorisvandenbossche fixed the conflicts, reformatted, and fixed the lint errors in https://github.com/pandas-dev/pandas/runs/161581700

@jorisvandenbossche
Copy link
Member Author

Ah, sorry, have them locally as well. Did you also fix the test_validate_docstrings error? That I am doing now (and also ran black on the full repo instead of only on /pandas)

@TomAugspurger
Copy link
Contributor

Ah, sorry. I missed the validate_docstrings one.

Feel free to force push.

@TomAugspurger
Copy link
Contributor

 class TimedeltaConstructor:
diff --git a/pandas/core/indexes/datetimelike.py b/pandas/core/indexes/datetimelike.py
index f2e6f631ae9..731ab9c4163 100644
--- a/pandas/core/indexes/datetimelike.py
+++ b/pandas/core/indexes/datetimelike.py
@@ -73,14 +73,14 @@ class DatetimeIndexOpsMixin(ExtensionOpsMixin):
     # properties there.  They can be made into cache_readonly for Index
     # subclasses bc they are immutable
     inferred_freq = cache_readonly(
-        DatetimeLikeArrayMixin.inferred_freq.fget
-    )  # type: ignore
+        DatetimeLikeArrayMixin.inferred_freq.fget  # type: ignore
+    )
     _isnan = cache_readonly(DatetimeLikeArrayMixin._isnan.fget)  # type: ignore
     hasnans = cache_readonly(DatetimeLikeArrayMixin._hasnans.fget)  # type: ignore
     _hasnans = hasnans  # for index / array -agnostic code
     _resolution = cache_readonly(
-        DatetimeLikeArrayMixin._resolution.fget
-    )  # type: ignore
+        DatetimeLikeArrayMixin._resolution.fget  # type: ignore
+    )
     resolution = cache_readonly(DatetimeLikeArrayMixin.resolution.fget)  # type: ignore
 
     _maybe_mask_results = ea_passthrough(DatetimeLikeArrayMixin._maybe_mask_results)

seemed to fix the typing issue.

@jorisvandenbossche
Copy link
Member Author

BTW, are you OK with re-enabling the flake8 check for whitespace after comma? I first put that in the exclude list, as black can generate that (eg in df.loc[1,] instead of df.loc[, ]). But, for the docs + docstrings, it is nice to keep that flake8 check strict.
In the code that black formats, it is only on a few lines that I need to add a noqa to have flake8 pass

@jorisvandenbossche
Copy link
Member Author

Up to now I kept separate commits for updating CI, the actual reformatting, and some fix-ups (because it made re-applying easier). Do we want to keep those separate when merging to keep the automated changes in a separate commit? (or it might be fine to have the separate commits here in the PR)

@TomAugspurger
Copy link
Contributor

Multiple commits (rebase and merge) seems fine.

@TomAugspurger
Copy link
Contributor

@jorisvandenbossche all green.

@jorisvandenbossche
Copy link
Member Author

I just rebased to update the commit titles and to combine the last two fixup commits (so they should be fine to rebase/merge). They should still be exactly the same content, so relying on green CI of the previous push.

@jorisvandenbossche jorisvandenbossche merged commit 8ea102a into pandas-dev:master Jul 4, 2019
@jorisvandenbossche jorisvandenbossche deleted the blacken branch July 4, 2019 03:28
@TomAugspurger
Copy link
Contributor

TomAugspurger commented Jul 4, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker Blocking issue or pull request for an upcoming release Code Style Code style, linting, code_checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants