PEP8 style: hanging indentation and closing brackets

pep8
Tags: #<Tag:0x00007f61a88a0280>

(Michael Wood-Vasey) #1

Which of the following three are preferred under our interpretation of PEP8?

To short-circuit the discussion: I naturally write #1. In part because I want to be able to minimize number of lines changed when adding and removing things from lists.

I’m OK with #2.

But only #3 passes PEP8 flake8 (with the RFC config file).

        kwargs = {
            'good_mag_limit': good_mag_limit,
            'medianAstromscatterRef': medianAstromscatterRef,
            'medianPhotoscatterRef': medianPhotoscatterRef,
            'matchRef': matchRef,
            }
        kwargs = {'good_mag_limit': good_mag_limit,
                  'medianAstromscatterRef': medianAstromscatterRef,
                  'medianPhotoscatterRef': medianPhotoscatterRef,
                  'matchRef': matchRef,
                 }
        kwargs = {'good_mag_limit': good_mag_limit,
                  'medianAstromscatterRef': medianAstromscatterRef,
                  'medianPhotoscatterRef': medianPhotoscatterRef,
                  'matchRef': matchRef}

The key issue is where to put the closing bracket. PEP8 is unhappy with the ways I think make the most sense.

Links of interest:
https://jira.lsstcorp.org/browse/RFC-162


(Jim Bosch) #2

I personally prefer (1), and I tend to use something similar for function calls or definitions that extend beyond one line. I think it’s the only one of these options that scales to the next level (when one of the dict values extends beyond one line, or you have a nested dict).

I prefer to put the closing brace at the same indentation as the variable definition, but I don’t care about that all that much:

    kwargs = {
        ...
    }

(Jonathan Sick) #3

Option #3 is absolutely what I’d use most of the time as a PEP8 purist.

If the names of your key value pairs are long then you can also do

kwargs = {
    'good_mag_limit': good_mag_limit,
    'medianAstromscatterRef': medianAstromscatterRef,
    'medianPhotoscatterRef': medianPhotoscatterRef,
    'matchRef': matchRef,
}

as @jbosch suggests. That’s also PEP8 compliant.


(Paul Price) #4

I think @jbosch likes:

kwargs = {
    'good_mag_limit': good_mag_limit,
    'medianAstromscatterRef': medianAstromscatterRef,
    'medianPhotoscatterRef': medianPhotoscatterRef,
    'matchRef': matchRef,
}

But that’s different from 1.


(Tim Jenness) #5

Agreed. +1 on (4). This is one of those things that flake8 can handle but is not enabled by default.


(Paul Price) #6

I’ve no great preference over where the closing brace goes (except emacs naturally does option #1 for me and it’s a pain to fix it). But I really don’t like the following:

kwargs = {
    'good_mag_limit':         good_mag_limit,
    'medianAstromscatterRef': medianAstromscatterRef,
    'medianPhotoscatterRef':  medianPhotoscatterRef,
    'matchRef':               matchRef,
} # for any placement of this brace

(John Swinbank) #7

:+1:. Failing that, (3) (the pure PEP-8 option).


(Michael Wood-Vasey) #8

Thanks, all. I take the adopted answer as (4):

kwargs = {
    'good_mag_limit': good_mag_limit,
    'medianAstromscatterRef': medianAstromscatterRef,
    'medianPhotoscatterRef': medianPhotoscatterRef,
    'matchRef': matchRef,
}

(John Parejko) #9

Wait, where does the indentation there go? I though it was supposed to align with the top brace (a la 2/3)?


(Jonathan Sick) #10

In case #4 you’re allowed to indent by 4 spaces from the first line of a multi-line statement.


(John Parejko) #11

By “allowed” here, you mean “required”?

I’m coming at this from a view that the PEP8 “# Aligned with opening delimiter.” style is correct and what we want.


(Jonathan Sick) #12

PEP 8 is fine with either #3 or #4. Both are useful. I tend to use #3 first, and fall back to #4 if I need the extra horizontal space.


(John Parejko) #13

Great. No complaints about that.