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:
-
we use half the time each of:
if foo==bar: if foo == bar:it should be the second way
-
we use:
foo=bar foo = barit should be the second way,
unless it’s a func argument, then it’s
func(foo=True, bar=False) -
we use:
func(foo,bar, tar) func(foo, bar, tar)it should be the second way
-
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) -
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.