-
Notifications
You must be signed in to change notification settings - Fork 67
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
PEP8 / Black style makes some code less readable #235
Comments
I think that black is NOT configurable (by design), but you can have it ignore blocks. This is from the documentation:
Black is a PEP 8 compliant opinionated formatter. Black reformats entire files in place. It is not configurable. It doesn't take previous formatting into account. Your main option of configuring Black is that it doesn't reformat blocks that start with # fmt: off and end with # fmt: on. # fmt: on/off have to be on the same level of indentation. To learn more about Black's opinions, to go the_black_code_style <https://github.com/psf/black/blob/master/docs/the_black_code_style.md>.
On Oct 14, 2020, at 1:55 PM, Paul T. Baker ***@***.***> wrote:
Complicated equations are made less, not more, readable by following the PEP8 style that black enforces.
You can see this type of thing in signals.utils, where some of the most complicated mathematical expressions live:
original
dFdt = 48 / (5*np.pi*mc**2) * (2*np.pi*mc*F)**(11/3) * \
(1 + 73/24*e**2 + 37/96*e**4) / ((1-e**2)**(7/2))
after black
dFdt = (
48
/ (5 * np.pi * mc ** 2)
* (2 * np.pi * mc * F) ** (11 / 3)
* (1 + 73 / 24 * e ** 2 + 37 / 96 * e ** 4)
/ ((1 - e ** 2) ** (7 / 2))
)
Does anyone know how to configure black to ignore certain rules?
The pre-black linter was configured to ignore "space around operator" errors, so things like
2*10**logA + B/C
wouldn't be flagged. The linter wanted it to be 2 * 10 ** logA + B / C which I find much harder to parse by eye.
another example
In PR #228 <#228> this block (which could be a bit better) was originally this:
return (
sum(
(params[efac.name] if efac.name in params else efac.value) * mask
for efac, mask in zip(self._dmefacs, self._dmefac_masks)
)**2
* self._dmerr**2 +
(10**sum(
(params[equad.name] if equad.name in params else equad.value) * mask
for equad, mask in zip(self._log10_dmequads, self._log10_dmequad_masks)
))**2
)**0.5
It becomes this (which is harder for me to follow):
return (
sum(
(params[efac.name] if efac.name in params else efac.value) * mask
for efac, mask in zip(self._dmefacs, self._dmefac_masks)
)
** 2
* self._dmerr ** 2
+ (
10
** sum(
(params[equad.name] if equad.name in params else equad.value) * mask
for equad, mask in zip(self._log10_dmequads, self._log10_dmequad_masks)
)
)
** 2
) ** 0.5
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#235>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUHXOTI3G4MKCR77AC4XK3SKXQYBANCNFSM4SQ5WTTQ>.
Kayhan Gültekin -- +1 (734) 255-7082
|
OMG, whoever wrote that return statement should be made to manually re-lint all the code..... That is atrocious (black or no black). |
I guess we could tell it to ignore whole files. At this point since I guess the solution is to break complicated equations into parts, storing individual terms in variables. That would mean the whole code would be built from simpler expressions. |
I didn't write it, but I merged it... so I'm possibly more to blame 🤦 |
Complicated equations are made less, not more, readable by following the PEP8 style that
black
enforces.You can see this type of thing in
signals.utils
, where some of the most complicated mathematical expressions live:original
after
black
Does anyone know how to configure
black
to ignore certain rules?The pre-
black
linter was configured to ignore "space around operator" errors, so things likewouldn't be flagged. The linter wanted it to be
2 * 10 ** logA + B / C
which I find much harder to parse by eye.another example
In PR #228 this block (which could be a bit better) was originally this:
It becomes this (which is harder for me to follow):
The text was updated successfully, but these errors were encountered: