Hacker News new | past | comments | ask | show | jobs | submit login

Ufff that's rough to read. Totally. The only one I agree with is the one that says:

    Aim to align statement parts that are conceptually similar. It allows the reader to quickly see how they’re different. E.g. in this code it’s immediately clear that the two parts call the same code with different parameter orders.
That'd turn something like this:

    class OneClass:
        def __init__(self, a, b1, b2, c_long):
            self.a = a
            self.b1 = b1
            self.b2 = b2
            self.c_long = c_long
Into this:

    class OneClass:
        def __init__(self, a, b1, b2, c_long):
            self.a      = a
            self.b1     = b1
            self.b2     = b2
            self.c_long = c_long
(maybe not the greatest example, there are places where this helps much more)

The others all are un-pythonic and make the code more unreadable.






It's certainly unpythonic - as the link explains, it's based on research that goes back many more decades than Python has existed, and that PEP 8 entirely ignored.

But it only makes the code unreadable if you don't make a tiny effort to adjust. If you do make the effort, there's some great payoff, like this code:

    try:
        self._split(b);                                  self('begin_batch')
        self.pred = self.model(*self.xb);                self('after_pred')
        if len(self.yb) == 0: return
        self.loss = self.loss_func(self.pred, *self.yb); self('after_loss')
        if not self.training: return
        self.loss.backward();                            self('after_backward')
        self.opt.step();                                 self('after_step')
        self.opt.zero_grad()
    except CancelBatchException:                         self('after_cancel_batch')
    finally:                                             self('after_batch')
That's the inner part of the training loop. You can see at a glance: what steps are in the loop; what callbacks are in the loop, in what order; what step corresponds to each callback. And you can see the whole training loop at once, which is great for getting a clear picture of what's going on.

By looking at that code without knowing much of the context in which it works, I assume self('begin_batch') and the like are "signals" that set some state or are used for logging. That behaviour could be achieved using other mechanisms (perhaps some metaprogramming magic or an observer pattern).

And while I can appreciate it can be quick to see where the signals are sent, the use of ; and having two things in a line still aren't convincing me.

Even more, if I were to run the line_profiler here, I know it'd report weird numbers precisely for having more than one thing per line.

The other thing that I dislike is opening blocks and closing them in the same line. It may be force of habit for me, but that screams unreadability at my face.

Rounding up, all I see is behaviour that can be achieved through other mechanisms, and dev/tools unfriendliness. And notice I'm not sayin anything about PEP-8, because:

a) There are parts of it with which I don't agree either.

b) Many people use PEP-8 as a sort of "silver bullet" and argument-ending-remark. That's not what it should be, it should be a _guide_ to be used when it helps, and ignored sparingly (with reason and consideration of _why_ you decide to ignore it, in the sake of readability).


I've long wished black would do this!

While I appreciate what black can do (no more discussions about code style!) I am lucky enough that I manage a small team (2-5 programmers) that understand and follow the style convention we set.



Applications are open for YC Summer 2020

Guidelines | FAQ | Support | API | Security | Lists | Bookmarklet | Legal | Apply to YC | Contact

Search: