[SciPy-Dev] Missing whitespace after commas in pycodestyle check

Ilhan Polat ilhanpolat at gmail.com
Mon Jan 13 18:15:00 EST 2020


I actually got pretty quick at fixing these stuff by the aid of some very
mild nonaggressive scripts. Hence if we all agree I can go through all repo
and fix everything once and ask the current PR owners to rebase. Sounds
like a chore to everyone but I guess biting the bullet once and then
turning on all the checks seems the least painful way. Otherwise Ralf's
suggestion is also a good idea.

On Mon, Jan 13, 2020 at 11:10 PM Ralf Gommers <ralf.gommers at gmail.com>
wrote:

> Hey Josh,
>
>
> On Mon, Jan 13, 2020 at 1:44 AM Joshua Wilson <josh.craig.wilson at gmail.com>
> wrote:
>
>> Hey everyone,
>>
>> During code review, it seems that we care about missing whitespace
>> after commas. (I have no data on this, but I suspect it is our most
>> common linting nit.) Linting for this is currently disabled. Now on
>> the one hand, we generally avoid turning on checks like this because
>> of the disruption to open PRs and what they do to git history. On the
>> other hand, I don't think enforcing this check by hand works.
>> Reminding an author to add spaces after commas can easily add several
>> rounds of extra review (some places almost invariably get missed on
>> the first try, and then the problem tends to sneak back in in later
>> commits), and I usually just give up after a while. So I don't think
>> we are heading in the direction of "in 5 years all the bad cases will
>> be gone and we can turn on the check without disruption".
>>
>
> I agree, the current workflow is not optimal.
>
>
>> So I would kind of like to push us to either:
>>
>> 1. Turn the lint check on. We can do this module-by-module or even
>> file-by-file if necessary to minimize disruption.
>> 2. Declare that this is not something we care about in code review.
>>
>
> Another option would be to run pycodestyle/pyflakes with all checks we
> want enabled, but only on the diff in the PR. This is what PyTorch does for
> example. The nice thing is you can test what you want without having to
> deal with code churn before enabling a check. The downside is that it's a
> bit convoluted to reproduce the "run on the diff to master" command locally
> (but we could add a script to streamline that probably).
>
> Cheers,
> Ralf
>
> _______________________________________________
> SciPy-Dev mailing list
> SciPy-Dev at python.org
> https://mail.python.org/mailman/listinfo/scipy-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/scipy-dev/attachments/20200114/94a21a3e/attachment-0001.html>


More information about the SciPy-Dev mailing list