-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
Use black 19.10b0 #29508
Use black 19.10b0 #29508
Conversation
res = sparse[4:,] # noqa: E231 | ||
res = sparse[ | ||
4:, | ||
] # noqa: E231 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is pretty gnarly. is there an alternative?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems related to: psf/black#1139
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to turn off formatting for a single line? (something like res = sparse[4:, ] # fmt: off
, but from their README it seems this only works for blocks)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was surprised that black doesn't support this - I upvoted -> psf/black#790
I suspect they will revert this in psf/black#1139
tm.assert_frame_equal(res, df.loc[df.A > 2,]) # noqa: E231 | ||
|
||
res = df.loc[lambda x: x.A > 2,] # noqa: E231 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't read any better either
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be nice explicitly specify the version of black
in the .pre-commit-config.yaml
:
pandas/.pre-commit-config.yaml
Lines 2 to 6 in 9bbb5f0
- repo: https://github.com/python/black | |
rev: stable | |
hooks: | |
- id: black | |
language_version: python3.7 |
Just a matter of modifying stable
--> 19.10b0
Good point - done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is backward incompatible? pls update the contributing guide as well
Added a note in contributing guide. Correct - backwards incompatible. i.e Running an older version will undo some of the changes here. Examples of the backwards incompatible issues can be found here: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
can you merge master. @TomAugspurger @jorisvandenbossche so this will force the new black in development. I think we are ok with this as its a dev dep. |
SGTM
…On Mon, Nov 11, 2019 at 8:36 AM Jeff Reback ***@***.***> wrote:
can you merge master.
@TomAugspurger <https://github.com/TomAugspurger> @jorisvandenbossche
<https://github.com/jorisvandenbossche>
so this will force the new black in development. I *think* we are ok with
this as its a dev dep.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#29508?email_source=notifications&email_token=AAKAOITKW2TM4PMTCMVJG3LQTFUVDA5CNFSM4JLGLBI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDXAMLY#issuecomment-552470063>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKAOISTA7SJDP7MJOIIIU3QTFUVDANCNFSM4JLGLBIQ>
.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me as well (if black changes output, there is no way around fixing this / pinning black versions more strictly)
I am only wondering if we should wait a bit longer with adopting black 19.10. It seems there are several open issues of regressions related to the "trailing comma in tuple" formatting (which gave some ugly / confusing reformatting in this PR). Given that feedback, they might further tweak that output (on the other hand, it's only a few occurrences in the pandas codebase, so can always easily change again later)
@@ -22,8 +22,8 @@ def test_index_col_named(all_parsers, with_header): | |||
KORD5,19990127, 22:00:00, 21:56:00, -0.5900, 1.7100, 5.1000, 0.0000, 290.0000 | |||
KORD6,19990127, 23:00:00, 22:56:00, -0.5900, 1.7100, 4.6000, 0.0000, 280.0000""" # noqa | |||
header = ( | |||
"ID,date,NominalTime,ActualTime,TDew,TAir,Windspeed,Precip,WindDir\n" | |||
) # noqa | |||
"ID,date,NominalTime,ActualTime,TDew,TAir,Windspeed,Precip,WindDir\n" # noqa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose this noqa is also not needed? (was needed before for too long line length, but that is no longer the case)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh I missed this - done
res = sparse[4:,] # noqa: E231 | ||
res = sparse[ | ||
4:, | ||
] # noqa: E231 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to turn off formatting for a single line? (something like res = sparse[4:, ] # fmt: off
, but from their README it seems this only works for blocks)
Merged master and addressed final comments. |
@jorisvandenbossche in reply to your comment: I was surprised that black doesn't support this - I upvoted -> psf/black#790 I hope they will address this soon psf/black#1139 |
Anything else needed here @jreback @jorisvandenbossche ? |
can you see if there is going to be a forthcoming releases in black. some of these changes might be reverted. I don't have a problem with fixing the version of black that we are using. but we shouldn't churn unecessarily. |
Sure it seems there will be a stable black release in 2019 (no more betas) - according to @ambv - here. psf/black#1139 isn't fixed on black master yet though - I imagine the only the changes made here that may be revert are to |
I am fine with merging this now, it's only a small change that we might need to revert again |
Hello @alimcmaster1! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2019-11-17 14:43:16 UTC |
Merged master and green. cc @jreback |
@@ -15,7 +15,7 @@ dependencies: | |||
- cython>=0.29.13 | |||
|
|||
# code checks | |||
- black<=19.3b0 | |||
- black>=19.10b0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make this ==
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure done here - #29673
thanks @alimcmaster1 |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
Main differences can be found on the 19.10 changelog:
https://black.readthedocs.io/en/stable/change_log.html