Molecule: yapf 0.27 breaks lint

[seandst]

Issue Type

• Bug report

Detail any linters or test runners used:

tox -e lint -e format-check

Desired Behavior

When using the latest yapf (currently 0.27), both the lint and format-check testenvs will succeed.

Actual Behaviour

When using the latest yapf on the current master, lint will succeed, but format-check will fail. If yapf if run with -i to modify files in-place (via tox -e format), format-check will succeed, but lint will fail.

This issue is being created as a central place to discuss how to proceed.

[seandst]

PR #1911 demonstrates the lint failures after the yapf's changes are applied.

[webknjaz]

/me still doesn't like autoformatters

[seandst]

I also don't like autoformatters. I think flake8 gets to have the final say on linting, and any personal style preferences can be hashed out in reviews.

This particular "yapf-ism" is probably my favorite, though I admit that I'm cherry-picking here:
https://github.com/ansible/molecule/blob/50688a810a77feaea4925d04e6d810b928adc39a/molecule/config.py#L404-L405

[seandst]

Ah, woops. I forgot to ref #1915, which pins yapf to 0.26 so CI has a better shot of working while this discussion happens.

[decentral1se]

any personal style preferences can be hashed out in reviews.

This is exactly the thing I like to avoid. And not even for myself but for contributors that might get exposed to some cranky "don't do it like this" comment and then think that this is not a welcoming place. There are big possibilities for that to happen ... overall, I think we need these tools because they are part of a equalising process for both new and old participants in this project. As as aside, I did like Knuth's comment (from the back of my head, no link right now ...) that we shouldn't restrict people based on what they find satisfying style ... but again, an overriding concern is contributor experience ...

Personally, I use black on all my projects (following projects like pytest and tox!) and it has been "set it and forget it" ... but I won't advocate for switching tools just yet. Especially since it is beta and experimental right now.

Let's just fix yapf for now ... I might take a stab at it ...

[webknjaz]

I use black

Oh, I can't even decide which one I dislike more :) I think black, because I have read its source code. They are probably fine for newbies but is poisonous for experienced folks.

The problem is that if I write code which is compliant with PEP 8 tools shouldn't break it but they do.

It's okay to format some parts which are not compliant, though, I'd be satisfied if they didn't touch well-written chunks and didn't break those. That's why I'm having "no-black" policy on my projects.

The other thing is that linter errors hint code author about possible places for improvement where the best way to solve them would be not blindly reformatting the chunk but redesigning it. This would save more resources of maintainers who still need to point out such places for refactoring but without any help from linters now...

[decentral1se]

They are probably fine for newbies but is poisonous for experienced folks.
That's why I'm having "no-black" policy on my projects.

I think you should re-evaluate your position if you're trying to make community projects more accessible.

[webknjaz]

you're trying to make community projects more accessible.

Nope, my goal is to keep them maintainable.

[seandst]

I don't have much to add beyond my initial experience as a relative newcomer to molecule. yapf itself resulted in a less-welcoming experience for me, as I initially did my work to flake8's satisfaction only to find that yapf disagreed with some of my style choices, and I in turn disagreed with yapf's choices. The experience was admittedly trivially annoying, but it was annoying nonetheless.

While taking the style friction out of PRs is laudable, it never really goes away. Using yapf moves the friction into a contributor's workspace, making it slightly more difficult to begin reasoned discussions about the merits of different approaches or the rationale behind specific style choices. Sometimes those discussions are frustrating, but more often (in my experience, at least) those discussions are rewarding and help me to become a more thoughtful developer.

[webknjaz]

yapf itself resulted in a less-welcoming experience for me, as I initially did my work to flake8's satisfaction only to find that yapf disagreed with some of my style choices, and I in turn disagreed with yapf's choices. The experience was admittedly trivially annoying

Yes, I think this is what I was trying to say... There's two sides of contributor experience (pros vs. newbies). And what makes one side happy just kills any joy and is frustrating others.

those discussions are rewarding and help me to become a more thoughtful developer.

Yeah, I like that too!

[seandst]

Some notes about this from today's working group meeting:

As I mentioned before, I'm only trivially frustrated with yapf. I don't deeply care how code ends up looking as long as it's still readable. With yapf currently pinned at 0.26, the lint and format-check testenvs get along with each other and work well together, so there's no urgency to make a decision now that CI is green again. It's possible that yapf will get updated in the meantime and resolve the flake8 incompatibility issue while this discussion continues.

[decentral1se]

Interesting data point on the code formatting debate: https://github.com/django/deps/pull/55/files

EDIT: Realise this is off-topic and don't want to further debate this but I appreciate that some of you on this thread and in and around the project might be interested in learning what others are saying about code formatting. That's all :)

[webknjaz]

I read that. Nothing new for me. I still hate when the computer tells me that my 100% compliant code should look not as I want it too (I wouldn't write it that way if I thought otherwise). That document just declares the subset of arguments the author doesn't like unimportant which is a very personal assessment.

[decentral1se]

:running_man: :bomb: