Black and pylint don't agree on bad-continuation

[evaherrada]

Pylint prefers this:

def enable_tap_detection(
            self,
            *,
            tap_count=1,
            threshold=25,
            long_initial_window=True,
            long_quiet_window=True,
            double_tap_window=TapDuration.DURATION_250_MS
    ):

Black prefers this:

def enable_tap_detection(
        self,
        *,
        tap_count=1,
        threshold=25,
        long_initial_window=True,
        long_quiet_window=True,
        double_tap_window=TapDuration.DURATION_250_MS
    ):

Personally, I think it'd be easier to just do a pylint disable there because the change seems pretty inconsequential, and I'm not sure how one would do a black disable or if it is even a thing.

Main issue:
adafruit/Adafruit_CircuitPython_Bundle#232

[kattni]

@dherrada FYI, from the Black documentation:
"Black reformats entire files in place. It is not configurable. It doesn't take previous formatting into account. It doesn't reformat blocks that start with # fmt: off and end with # fmt: on. # fmt: on/off have to be on the same level of indentation."

[evaherrada]

Thanks, that's good to know

[kattni]

@dherrada I don't want to default to disabling Pylint even if the change seems inconsequential. Please look into why Pylint and Black prefer the different spacing, so we can begin to discuss which to go with. This seems like an issue you will likely continue to run into. We should make an informed decision.

[kattni]

@dherrada What is the actual pylint failure? Is it bad-continuation?

[kattni]

@dherrada Please research this for half an hour and then report back at that time with what you've found. If you haven't found anything at that point, we'll work together on it.

[evaherrada]

@kattni Yeah, it is bad-continuation. Looking at black's docs right now

[evaherrada]

@kattni So from what I've gathered, PEP8 is fine with either style:

When using a hanging indent the following should be considered; there should be no arguments on the first line and further indentation should be used to clearly distinguish itself as a continuation line.

In this case, one indent in from the line with the ): is enough to clearly distinguish it, which would mean that pylint is technically in the wrong on this one by saying the formatting black prefers is incorrect. Looks like this is a really common problem when using black and pylint.

pylint-dev/pylint#289
psf/black#48

[kattni]

Excellent. Great job finding that! We are under no circumstances going to disable bad-continuation globally, so please disable it for that block only in this library.

[evaherrada]

@kattni Would you like me to push and make a pr?

[kattni]

@dherrada We haven't patched the libs with the easy way to update the version of Pylint being used. Will it still pass 1.9.2 with that change?

[evaherrada]

@kattni Yep, it will

[kattni]

@dherrada Perfect. Yes please push and make a PR.

[kattni]

Updated title to include actual issue. @dherrada Please make sure the titles of these issues are more descriptive moving forward so when we look at the master issue on the Bundle, it will be clearer what we've discussed already.

[evaherrada]

@kattni I'll make sure to do that

[tannewt]

FYI I already disabled bad-continuation in cookiecutter because it conflicts with pylint: https://github.com/adafruit/cookiecutter-adafruit-circuitpython/pull/64/files

[kattni]

@tannewt I don't agree with disabling it globally. There are plenty of situations where Pylint finds other instances of bad continuation. I realise there is the bug in Pylint where it disagrees with Black, but I question whether we should be handling this locally in that single instance and let Pylint continue to find it in other instances.

[tannewt]

Do you have examples where pylint finds bad continuations that black doesn't? I assume that black handles all of that sort of formatting.

[kattni]

I don't. If Black handles all of it, then I guess we can disable it.

@dherrada Please submit a PR to this repo to add bad-continuation to the .pylintrc file, and remove the local pylint: disable= from the library. Please update your currently open PRs to include bad-continuation in the .pylintrc as well, and remove any local disables you may have included. We will do this manually for now as you update the libraries.

[evaherrada]

@kattni Ok. I've done that

[evaherrada]

@kattni I can close this issue, correct?

[kattni]

@dherrada Correct.

[kattni]

@tannewt Moving forward, if you intend to update the .pylintrc globally on cookiecutter, please file an issue on the cookiecutter repo and tag me so I am aware of the changes, and so I understand why the changes are being made. Thank you.

[tannewt]

Ok will do @kattni. I have nothing else in the pipeline.