getavalon / core

The safe post-production pipeline - https://getavalon.github.io/2.0
MIT License
213 stars 48 forks source link

Fix hound with a bit better flake8 setup #557

Closed davidlatwe closed 3 years ago

davidlatwe commented 3 years ago

What's changed

Discussion

Which should we follow ?

1) Write with line break before binary op (W503)

foo = ("hello"
       + "world,"
       + "bar")

or,

2) write line break after binary op (W504)

foo = ("hello" +
       "world," +
       "bar")

I now like to break before (option 1), since I can view all the operations in a aligned stack, what do you guys think ?

Additional notes

How to invoke flake8 on local

$ pip install flake8
$ cd avalon-core
$ flake8
tokejepsen commented 3 years ago

Personally I would vote for option 2 because its easier to read.

davidlatwe commented 3 years ago

Hmmm, I remember that Pype club also take option 2 as well, any other inputs ? @mottosso @BigRoy

mottosso commented 3 years ago

Needs more examples to tell. With the plain-text example, option 2 looks better. Because all operators are the same. But with more mathy stuff, I tend to prefer 1.

# Option 1
rotation = (some_matrix
            * Vector3(1, 0, 0)
            / 0.12
            % 2)

# Option 2
rotation = (some_matrix *
            Vector3(1, 0, 0) /
            0.12 %
            2)

Though I would probably add one more line, so as to not depend on the length of the variable name.

rotation = (
    some_matrix
    * Vector3(1, 0, 0)
    / 0.12
    % 2
)

translation_with_longer_name = (
    some_matrix
    * Vector2(1, 0, 0)
    / 613.1
    * 100
)

Since we can only have one option for all occurrences, and since strings don't technically need the + anyway, I'd vote for option 1. 👍

davidlatwe commented 3 years ago

Good example !

jasperges commented 3 years ago

Personally I prefer the linebreak before the a binary operator, because you can immediately see the operator. https://www.python.org/dev/peps/pep-0008/#should-a-line-break-before-or-after-a-binary-operator

Not necessarily a good reason, but Black also does it like this. At first I wasn't a fan of Black, but now I quite like it actually. It makes your code formatting consistent.

BigRoy commented 3 years ago

I'm honestly fine with either. I'd be inclined to go with linebreak before the a binary operator if that's PEP8. However, I don't necessarily prefer one over the other.

As it's currently a tie (if pype really uses option 2) I'll have them add an opinion directly @antirotor @mkolar @iLLiCiTiT

Then I guess we should just pull the trigger. :)

iLLiCiTiT commented 3 years ago

I'll have them add an opinion directly @antirotor @mkolar @iLLiCiTiT

I think we have different opinions about that as well. We've agreed to have binary operator before as you can easily see how the argument affect result.

I personaly also preffer to add one more line to not depend on line length of variable name, the one more line enhance readability of the content for me.

antirotor commented 3 years ago

It is also possible to merge all kinds of config files into one - setup.cfg. There you can create [flake8] section and rest is the same. There can also be coverage, pytest, tox and most of other similar tools, reducing amount of files in repository root. Just for consideration...

mkolar commented 3 years ago

This got hanging here a bit :). So I'll add my two cents. I also prefer line break before binary operators. For the same reasons mentioned already. I find it somewhat easier to quickly scan the code and see what's happening with that.

davidlatwe commented 3 years ago

Line break before binary op it is ! Let's merge this !!