Coding Style for v1

Observing our PR review notes (mine and others), In an effort to be less of a hypocrite when asking PR submitters to adhere to our code style, and so that our code can act as monkey-see-monkey-do style guide, we need to sort out some inconsistencies:

Currently inconsistent style:

  1. we use half the time each of:

    if foo==bar:
    if foo == bar:
    

    it should be the second way

  2. we use:

    foo=bar
    foo = bar
    

    it should be the second way,

    unless it’s a func argument, then it’s

    func(foo=True, bar=False)
    
  3. we use:

    func(foo,bar, tar)
    func(foo, bar, tar)
    

    it should be the second way

  4. since we seem to need to stick with: func(foo, bar, tar), there is a conflict with these 2 styles:

    foo,bar,tar = a,b,c
    foo,bar,tar = func(a,b,c)
    

    since it should be:

    foo,bar,tar = func(a, b, c)
    

    but then this looks again inconsistent:

    foo,bar,tar = a,b,c
    foo,bar,tar = func(a, b, c)
    for i,x in zip(foo, bar)
    
  5. the style says:

    f"this {foo} bar"
    

    but we still have:

    "this "+ foo + " bar"
    

I just applied these changes to: https://github.com/fastai/fastai/commit/6c71bcafeda64b15b9961c3fbfbcd34af82c16fa

the only thing I don’t know how to resolve is the conflict between 2 styles in item 4 above.

update: I run these by Jeremy and he clarified that all these are good and there is no strict requirement other than what’s in the style guide: https://docs.fast.ai/dev/style.html, which doesn’t include any of the items I listed in this post.

So consider the above Stas’ style guide and this is how I write my code, but you can do it in any other way while adhering to the style guide (link above).

and I sorted out case 4 for my own preferences - func args are always space separated and the only time assignment has no space is when groups are assigned (not results of func) and in loops:

foo, bar = func(a, b, c)
foo,bar = a,b
foo,bar = funca(),funcb() # don't do it
foo,bar = funca(a, b, c),func(1, 2, 3) # neither this
for i,x in zip(foo, bar)

no more conflict there.

2 Likes