PEP8 / Black style makes some code less readable

[paulthebaker]

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 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

[kayhangultekin]

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 notifications@github.com 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 https://github.com/nanograv/enterprise/pull/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._dmerr2 + (10sum( (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 https://github.com/nanograv/enterprise/issues/235, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADUHXOTI3G4MKCR77AC4XK3SKXQYBANCNFSM4SQ5WTTQ.

Kayhan Gültekin -- +1 (734) 255-7082

[scottransom]

OMG, whoever wrote that return statement should be made to manually re-lint all the code..... That is atrocious (black or no black).

[paulthebaker]

I guess we could tell it to ignore whole files. At this point since black reformatted everything, it'd be a huge pain to manually re-lint things.

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.

[paulthebaker]

OMG, whoever wrote that return statement should be made to manually re-lint all the code..... That is atrocious (black or no black).

I didn't write it, but I merged it... so I'm possibly more to blame 🤦