Add Darken action to check for pep8 violations in diffs

[IAlibay]

Fixes #3854

I'm trying to like Black @RMeli 🙃

Changes made in this Pull Request:

• Darken action
• Some action cleanup (might split stuff up in a future PR so we can have a bit more coherence)

PR Checklist

• Tests?
• Docs?
• CHANGELOG updated?
• Issue raised/referenced?

[IAlibay]

There we go, so it ignores blank lines, but not 79 char issues... it's a start.

[IAlibay]

Ok looks like it's keeping track of the head branch properly, now to see if we can get it to complain on incorrect number of lines between class definitions.

[codecov]

Codecov Report

Base: 94.38% // Head: 94.41% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (46233f3) compared to base (3712439).
Patch has no changes to coverable lines.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3954      +/-   ##
===========================================
+ Coverage    94.38%   94.41%   +0.02%     
===========================================
  Files          194      194              
  Lines        25242    25249       +7     
  Branches      3493     3493              
===========================================
+ Hits         23825    23838      +13     
+ Misses        1368     1367       -1     
+ Partials        49       44       -5     

Impacted Files	Coverage Δ
package/MDAnalysis/lib/distances.py	98.51% <0.00%> (ø)
MDAnalysisTests/coordinates/base.py	94.46% <0.00%> (+<0.01%)	⬆️
package/MDAnalysis/analysis/align.py	98.35% <0.00%> (+0.03%)	⬆️
package/MDAnalysis/core/topologyobjects.py	98.92% <0.00%> (+0.35%)	⬆️
package/MDAnalysis/converters/RDKitParser.py	97.72% <0.00%> (+0.75%)	⬆️
package/MDAnalysis/converters/RDKit.py	100.00% <0.00%> (+1.25%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[IAlibay]

Ok this one is going to be contentious @MDAnalysis/coredevs so please take the time to review this before we have complaints further down the line.

What does this do?

• This action essentially applies Black and flake8 on the diff of a PR's commit. This means it'll tell you if it thinks you're doing shady PEP8 things

Why might it be annoying?

• Because it uses Black, it will have opinions on how things should be, particularly when it comes to visual indent for lists / dictionaries. Whilst generally useful, those suggestions might not agree with what you think is reasonable.
• Both black and flake8 actually go beyond PEP8, you might not agree with certain things...

Do we have to listen to it?

• Because I can't just get it to just always go green and just write out a comment (there's some security issues and I couldn't circumvent them), it'll just claim to be a failed runner.
• My proposal is that we don't treat it as the only truth. Read the failure, see if you agree, tell contributors to fix them, or don't, as long we pass your interpretation of PEP8 it's up to you!

What can't it do?

Currently my only observations are:

• The action won't be able to tell you off if you just spuriously add blank lines in certain places. Because it acts on the diff, it can't tell if you're adding more / less blank lines in places you shouldn't.
• Sometimes black will fail to identify lines going wrong, but flake8 picks up the slack.

[IAlibay]

P.S. This replaces our reliance on the unfortunately broken but amazing pep8speaks bot :(

[jbarnoud]

Could you make some changes that would not follow pep8 so we get to see what darken looks like?

I liked pep8speak, too bad it is broken 😢

[IAlibay]

Could you make some changes that would not follow pep8 so we get to see what darken looks like?

I liked pep8speak, too bad it is broken 😢

:D forgive me for my sins o Python gods (sure)

[jbarnoud]

The report is a bit dry, but I can do with it. What is the line length policy?

[orbeckst]

I can live with it.

[orbeckst]

For posterity: this is what output looks like for well/badly formatted code: https://github.com/MDAnalysis/mdanalysis/actions/runs/3651330936/jobs/6168426745

[orbeckst]

@IAlibay if you need a LGTM from me, remove the demonstration code and let me know.

[IAlibay]

@IAlibay if you need a LGTM from me, remove the demonstration code and let me know.

Should be good to go now @orbeckst

[orbeckst]

lgtm

[IAlibay]

The report is a bit dry, but I can do with it. What is the line length policy?

88 with darken, 79 with flake8, I'll see if there's a way to make them match

[IAlibay]

blergh the Black release that just came out earlier today is causing trouble

[IAlibay]

The report is a bit dry, but I can do with it. What is the line length policy?

88 with darken, 79 with flake8, I'll see if there's a way to make them match

@jbarnoud I've set them both to be 88 characters now.

[jbarnoud]

Lgtm