add black
#8
add black
#8
Conversation
There was a problem hiding this comment.
I've marked my main concerns with Black inline. The whitespace and quotation mark fixes are fine; the main concern is line breaks. If it could respect my choice of hanging indents and allow more than one list item per line, I might be convinced. I find it hard to navigate files when lists take up an entire screen like this.
| '__or__', '__pow__', '__rshift__', '__sub__', '__truediv__', | ||
| '__xor__'] + ['__divmod__', '__floordiv__'] | ||
| binary_names = [ | ||
| "__add__", |
There was a problem hiding this comment.
This is one of the things I am not a fan of. Perhaps if there is a rule that can be turned off, I can probably live with it.
| out = ( | ||
| MaskedArray(res, mask) | ||
| if name not in output_arrays | ||
| else [MaskedArray(resi, maski) for resi, maski in zip(res, mask)] | ||
| ) |
There was a problem hiding this comment.
Similarly here. I preferred the initial formatting of this ternary.
| def iadd(x, y): | ||
| x += y | ||
|
|
||
|
|
There was a problem hiding this comment.
And these one-liners just don't need four lines, IMO.
There was a problem hiding this comment.
I wonder, would it be fine to just use the functions in operator instead?
(in a separate PR, that doesn't have anything to do with code style)
There was a problem hiding this comment.
Maybe. I think I had a bug with the in-place operators anyway, so I'll investigate.
There was a problem hiding this comment.
black intentionally doesn't support options, so it's all-or-nothing.
The reasoning for the multi-line lists is that it allows modifications to be a single line diff instead of multiple / the modification of a long line.
For ternary operators I would actually write it like black did: each part of the ternary on a separate line.
I guess code style is a matter of habits and taste (personal preference), and thus often something hard to argue for / against. Again, the big plus of black is consistency rather than following everyone's personal preference perfectly; it is following more of a "mean denominator", if that makes sense.
| def iadd(x, y): | ||
| x += y | ||
|
|
||
|
|
There was a problem hiding this comment.
I wonder, would it be fine to just use the functions in operator instead?
(in a separate PR, that doesn't have anything to do with code style)
As mentioned in #7 (comment), this adds
blackto the hooks and "blackens" the existing code. Again, feel free to close if the changes are too much of an issue.(If we do decide to merge, though, I'd like to first add be2151b to the set of commits ignored by
git blameto avoid interfering with that)