STYLE: Reformat missing dir #761
Conversation
Softagram Impact Report for pull/761 (head commit: 6d78413)⭐ Change Overview
📄 Full report
Impact Report explained. Give feedback on this report to [email protected] |
Codecov Report
@@ Coverage Diff @@
## master #761 +/- ##
=======================================
Coverage 93.91% 93.91%
=======================================
Files 32 32
Lines 5667 5667
=======================================
Hits 5322 5322
Misses 345 345
Continue to review full report at Codecov.
|
|
|
||
| # Functions | ||
| agg = unsupported_function('agg') |
There was a problem hiding this comment.
Hm, '' vs "`. both are legitimate per PEP 8. Does this show an error?
There was a problem hiding this comment.
yeah, they standardize quotes to " in Black. You are right, both are legitimate in PEP8. I think they just try to avoid some people use ' while some use "
| def unsupported_function(*args, **kwargs): | ||
| raise PandasNotImplementedError(class_name=class_name, method_name=method_name, | ||
| reason=reason) |
There was a problem hiding this comment.
Hmm .. this is legitimate per PEP 8 if I didn't miss anything.
There was a problem hiding this comment.
yeah, i think almost all lines in the current codebase are legitimate. This is just how Black reformat codes to standardize code style i think
| reason="Unlike pandas, most DataFrames are not materialized in memory in Spark " | ||
| "(and Koalas), and as a result memory_usage() does not do what you intend it " | ||
| "to do. Use Spark's web UI to monitor disk and memory usage of your application.") | ||
| "(and Koalas), and as a result memory_usage() does not do what you intend it " |
There was a problem hiding this comment.
and, this actually looks a bit odd. Because alined indentation within parentheses is legitimate per PEP 8 as well (i use this style quite often too)
|
@charlesdong1991, is it possible to make the changes smaller by ignoring some rules? Seems it's too aggressive. |
|
yeah, i agree with you some look a bit odd and aggressive. There are several ways to write codes aligned with PEP8, and I think Black just choose one of them so that in a way all codes follow the same style. I will take a deeper look later to see if there are less aggressive way to figure this out. @HyukjinKwon with my past experience, i think sometimes code reformatted by Black is not aligned with btw, I think some of your doubts can be answered in their README, https://github.com/psf/black, and pls let me know your thoughts :) |
|
Hmmmm .. is it possible to disable the formatting itself less aggressively? |
|
or .. is it possible to only reformat the github diff optionally? In case of Apache Spark, it has a script called |
|
I'd prefer to only reformat the github diff, but seems like black does not support partial format (psf/black#320). |
i think only forcing the string quotes can be easily ignored in Black... |
|
sorry, i was away for a holiday, and back now, I am not sure how you want to proceed on this? or do you want to proceed? maybe give some directions you prefer? @HyukjinKwon @ueshin otherwise, i would like to pick up other issues then :) |
|
Can we only apply the formatting for github diff? I think @ueshin gave a pointer at #761 (comment) |
|
i am afraid partial formatting is not supported by black because it goes against black's philosophy which is to have standard/unified code format in the project even it's aligned with PEP8. Regarding the alternative, i also saw that repo quite long ago, and I don't know exactly how that works, but when i read it, in README it says: |
|
Hm .. let's drop this for now then. I think it's too aggressive and forces us the style even if you don't like it .. |
this pr will reformat
missingdir, and max length has set to 100.