PEP8 style: hanging indentation and closing brackets

Data Management
#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

#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 = {
        ...
    }
1 Like
#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.

#4

I think @jbosch likes:

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

But that’s different from 1.

2 Likes
#5

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

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

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

1 Like
#8

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

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

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

#10

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

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

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

#13

Great. No complaints about that.