Ticket #2916 (closed defect: fixed)

Opened 6 years ago

Last modified 6 years ago

Standards: Spaces around '=' in keyword arguments

Reported by: robyn Owned by: rowen
Priority: normal Milestone:
Component: TCT Keywords:
Cc: robyn, mjuric, ktl, rhl, smm, rowen, RayPlante, gpdf, mfreemon Blocked By:
Blocking: Project: LSST
Version Number:
How to repeat:

not applicable

Description

Robert Lupton the Good via lsstcorp.org wrote on May 30, 2013: In a review of #2904, Paul notes:

I believe keyword arguments are not supposed to have a space either side of the equals sign; but you're just following the existing style.

The code in question was setting configs:

doSetBadRegions = pexConfig.Field(

dtype = bool, doc = "Should we set the level of all BAD patches of the chip to the chip's average value?", default = True, )

Paul's correct; keyword arguments are not supposed to have spaces. But I'd argue that this was a special case. Yes; those are keyword arguments but the code is idiomatically setting config params and it's much closer in feel to setting variables (with spaces).

I propose that we permit (and require) spaces in this context. Thoughts?

Change History

comment:1 Changed 6 years ago by robyn

Jim Bosch replied May 30 2013:
to Robert, LSST

I *really* don't have a strong opinion on this at all, but I think it generally looks nicest when spaces are used when there's one keyword argument per line, and no spaces when they're all on the same line.

Jim


Russell Owen replied May 30, 2013
to Robert, LSST

I agree. I prefer to use no spaces around "=" for multiple arguments on one line, spaces for one argument per line. I feel it reads nicely.

That said, it's a small detail and I can live with the current standard or whatever variant we agree on. The next TCT meeting is coming up, so we could easily vote on a small change. Do you want to propose a specific wording?


comment:2 Changed 6 years ago by rowen

One more thing: if we change the rule, please keep it simple. I don't think it is realistic to expect users to decide whether spaces are wanted based on context (Config field yes, but how about constructing a Struct, calling other constructors, calling methods or free functions). If we change it, I suggest we encourage spaces around equals when we have one argument per line and discourage spaces when we have multiple arguments on one line. It is simple and makes for very readable code.

comment:3 Changed 6 years ago by srp

From the style guide:

http://www.python.org/dev/peps/pep-0008/

Don't use spaces around the = sign when used to indicate a keyword argument or a default parameter value.

Yes:

def complex(real, imag=0.0):

return magic(r=real, i=imag)

No:

def complex(real, imag = 0.0):

return magic(r = real, i = imag)

Search for "don't use" on that page to find the context.

I would say no spaces since it is in the style guide, and we should try and remain consistent to that. It's likely other users and contributors to our code would be using that style guide, rather than one with our own caveats.

comment:4 Changed 6 years ago by ktl

I think it's simplest to retain the existing "no spaces". It also serves to remind that the Config parameter definition is expressed as Field constructor arguments. For me, this doesn't reach the activation barrier to change existing policy.

comment:5 Changed 6 years ago by robyn

From: Robert Lupton the Good; Date: Wed, 5 Jun 2013 14:53:52 -0400

1.  I vote that we allow white space around '=' in configs.  I don't =
want to spend more time on code standards than necessary!

comment:6 Changed 6 years ago by rowen

I am strongly against special-casing configs. It's too hard to justify, in my opinion, and doesn't scale well.

I would be much happier with any of the following three options (in decreasing order of preference, but any is fine):

  • Recommend spaces around = when one assignment appears on a line, and recommend against space when multiple assignments appear on one line (i.e. most function calls).
  • Ditch the rule. Let people do as they like. I don't think readability suffers that much, and if so, maybe it's not worth fussing about.
  • Keep the current rule.

comment:7 Changed 6 years ago by robyn

  • Cc mfreemon added

This proposal was discussed at the TCT Meeting of 30 January 2014 attended by Russell Owen, K-T Lim, Robert Lupton, Mario Juric, and Jim Bosch. Mike Freemon sent his regrets since he had no opinion to offer on any of the proposals being reviewed. Gregory Dubois Felsmann sent his regrets but sent his proxy to K-T.

This proposal had a spirited and rapid paced discussion. The Scribe was unable to keep up the note taking.

  • There was discussion on providing an overall default proposal and then tacking on the various exceptions to the rule.
    • This was deemed to be more confusing that letting common sense reign.
  • There was discussion on whether to allow or disallow spaces when an argument list is formatted to contain only one 'a = b' construct per line or when it is formatted to have all arguments on a single long line (a=b,b=c,....).
    • This was deemed as more specific than warranted.
  • There was a comment that some restrictive Standards are making the code too hard to read.

The final and accepted proposal is being written up by Russell. However the gist of the proposal is:

Outside of an argument list, always surround the '=' with a space; inside of an argument list, the developer is urged to format for readability.

Post meeting note: It is hoped by the Chair that Russell will guide the reader towards readable code by providing samples addressing this Rule.

comment:8 Changed 6 years ago by robyn

  • Status changed from new to assigned
  • Owner changed from rhl to rowen

comment:9 Changed 6 years ago by rowen

  • Status changed from assigned to closed
  • Resolution set to fixed

I sent Robyn text and I believe she has incorporated it into the standards.

Note: See TracTickets for help on using tickets.