Some of the "missing" checks can be added by flake8 plugins -- now obviously the plugins don't always provide the "no-false-positives" guarantees

<https://github.com/PyCQA/flake8-bugbear> is a good one, and is part of the PyCQA so is "blessed".

There are many more possible ones we can look at on <https://github.com/DmytroLitvinov/awesome-flake8-extensions>, but I think we should keep the plugins small and only add specific plugins when we identify a need -- to keep flake8 running fast.

-ash

On Thu, Jun 24 2021 at 10:59:58 +0200, Jarek Potiuk <ja...@potiuk.com> wrote:
Added counts to get better overview:

grep -R '# pylint: disable' | sed 's/.*\(# pylint.*\)/\1/' | sort | uniq -c
      1 Binary file
.git/objects/pack/pack-b85cbf9c91a5ba2bc0e0f19ea9bcf9d8193c48d5.pack
matches
      3 # pylint: disable=abstract-method
     18 # pylint: disable=assignment-from-no-return
     22 # pylint: disable=attribute-defined-outside-init
    195 # pylint: disable=broad-except
      2 # pylint: disable=C0103
      2 # pylint: disable=C0103, line-too-long
      1 # pylint: disable=C0111
      2 # pylint: disable=C0123
      4 # pylint: disable=C0301
      3 # pylint: disable=C0302
     18 # pylint: disable=c-extension-no-member
      7 # pylint: disable=c-extension-no-member, no-member
      2 # pylint: disable=c-extension-no-member,no-member
     19 # pylint: disable=comparison-with-callable
      1 # pylint: disable=comparison-with-itself
      4 # pylint: disable=consider-using-enumerate
      2 # pylint: disable=consider-using-generator
     46 # pylint: disable=consider-using-with
2 # pylint: disable=consider-using-with,attribute-defined-outside-init
      4 # pylint: disable=cyclic-import
      2 # pylint: disable=dangerous-default-value
      2 # pylint: disable=E0012
      1 # pylint: disable=E1101
      2 # pylint: disable=E1102
      3 # pylint: disable=E1111
      8 # pylint: disable=expression-not-assigned
     29 # pylint: disable=global-statement
1 # pylint: disable=global-statement,global-variable-not-assigned
      4 # pylint: disable=inconsistent-return-statements
      1 # pylint: disable=invalid-bool-returned, bad-option-value
      1 # pylint: disable=invalid-length-returned
    123 # pylint: disable=invalid-name
      2 # pylint: disable=invalid-name,missing-docstring
      2 # pylint: disable=invalid-name, unused-argument
      1 # pylint: disable=invalid-sequence-index
      2 # pylint: disable=invalid-str-returned
      2 # pylint: disable=len-as-condition
     20 # pylint: disable=line-too-long
      3 # pylint: disable=line-too-long # noqa
      4 # pylint: disable=logging-not-lazy
      4 # pylint: disable=lost-exception
     26 # pylint: disable=maybe-no-member
     21 # pylint: disable=missing-docstring
      1 # pylint: disable=missing-docstring, redefined-outer-name
     12 # pylint: disable=missing-function-docstring
      6 # pylint: disable=missing-kwoa
      4 # pylint: disable-msg=too-many-arguments
      2 # pylint: disable=no-else-break
      2 # pylint: disable=no-else-continue
    741 # pylint: disable=no-member
      1 # pylint: disable=no-member,invalid-name
      3 # pylint: disable=no-member,line-too-long
      1 # pylint: disable=no-member, maybe-no-member
      3 # pylint: disable=no-member # noqa
      2 # pylint: disable=no-method-argument
     58 # pylint: disable=no-name-in-module
      1 # pylint: disable=no-name-in-module,wrong-import-order
      2 # pylint: disable=non-parent-init-called
      6 # pylint: disable=no-self-argument
      1 # pylint: disable=not-a-mapping
      6 # pylint: disable=not-callable
    222 # pylint: disable=no-value-for-parameter
      6 # pylint: disable=pointless-statement
    123 # pylint: disable=protected-access
2 # pylint: disable=protected-access,c-extension-no-member,no-member
      2 # pylint: disable=protected-access,no-member
      2 # pylint: disable=R0401
      2 # pylint: disable=R0902,R0904
      1 # pylint: disable=R0904, C0111
      1 # pylint: disable=R0904, C0111, C0302
      3 # pylint: disable=R0904, C0302
      3 # pylint: disable=R0913, C0302
      2 # pylint: disable=raising-format-tuple
     14 # pylint: disable=redefined-builtin
      1 # pylint: disable=redefined-builtin,unused-argument
2 # pylint: disable=redefined-builtin,unused-argument,too-many-arguments
     14 # pylint: disable=redefined-outer-name
      6 # pylint: disable=redefined-outer-name,reimported
7 # pylint: disable=redefined-outer-name,reimported,unused-import
      1 # pylint: disable=redefined-outer-name,unused-argument
2 # pylint: disable=reimported,unused-import,redefined-outer-name
      2 # pylint: disable=self-assigning-variable
     13 # pylint: disable=signature-differs
      2 # pylint: disable=signature-differs   # noqa: D403
      2 # pylint: disable=singleton-comparison
      4 # pylint: disable=subprocess-popen-preexec-fn
2 # pylint: disable=subprocess-popen-preexec-fn,consider-using-with
      5 # pylint: disable=subprocess-run-check
      7 # pylint: disable=super-init-not-called
      2 # pylint: disable=syntax-error
      3 # pylint: disable=too-few-public-methods
     17 # pylint: disable=too-many-ancestors
    270 # pylint: disable=too-many-arguments
2 # pylint: disable=too-many-arguments,inconsistent-return-statements
      6 # pylint: disable=too-many-arguments, too-many-locals
     26 # pylint: disable=too-many-arguments,too-many-locals
2 # pylint: disable=too-many-arguments,too-many-locals,too-many-branches
      2 # pylint: disable=too-many-arguments,too-many-locals,
too-many-statements
      2 # pylint: disable=too-many-boolean-expressions
      4 # pylint: disable=too-many-branches
      1 # pylint: disable=too-many-function-args
     78 # pylint: disable=too-many-instance-attributes
2 # pylint: disable=too-many-instance-attributes, too-many-arguments
      2 # pylint:
disable=too-many-instance-attributes,too-many-arguments,too-many-branches
      2 # pylint: disable=too-many-instance-attributes,too-many-locals
2 # pylint: disable=too-many-instance-attributes,too-many-public-methods
     14 # pylint: disable=too-many-lines
     21 # pylint: disable=too-many-locals
      6 # pylint: disable=too-many-locals,too-many-arguments
3 # pylint: disable=too-many-locals,too-many-arguments,invalid-name 3 # pylint: disable=too-many-locals,too-many-arguments, too-many-branches
      4 # pylint: disable=too-many-locals,too-many-statements
     71 # pylint: disable=too-many-nested-blocks
      9 # pylint: disable=too-many-public-methods
     18 # pylint: disable=too-many-return-statements
      7 # pylint: disable=too-many-statements
      5 # pylint: disable=undefined-variable
     11 # pylint: disable=unexpected-keyword-arg
      4 # pylint: disable=ungrouped-imports
      4 # pylint: disable=unidiomatic-typecheck
      1 # pylint: disable=unnecessary-lambda
     18 # pylint: disable=unsubscriptable-object
      4 # pylint: disable=unsupported-membership-test
      1 # pylint: disable = unused-argument
    188 # pylint: disable=unused-argument
      1 # pylint: disable=unused-argument,invalid-name
      1 # pylint: disable=unused-argument,line-too-long
      2 # pylint: disable=unused-argument # pragma: no cover
3 # pylint: disable=unused-argument,too-many-arguments,too-many-locals
      2 # pylint: disable=unused-argument, unused-variable
    549 # pylint: disable=unused-import
      2 # pylint: disable=unused-import,line-too-long
      2 # pylint: disable=unused-import # noqa
      2 # pylint: disable=unused-import  # noqa: F401
      4 # pylint: disable=unused-import # noqa: F401
      8 # pylint: disable=unused-variable
      1 # pylint: disable=unused-variable  # noqa
      4 # pylint: disable=using-constant-test
      1 # pylint: disable=W0104
      6 # pylint: disable=W0106
      1 # pylint: disable=W0108
      1 # pylint: disable=W0143
      2 # pylint: disable=W0201
     20 # pylint: disable=W0212
      1 # pylint: disable=W0612
      4 # pylint: disable=W0703
      1 # pylint: disable=wildcard-import
      5 # pylint: disable=wrong-import-order
      5 # pylint: disable=wrong-import-position
On Thu, Jun 24, 2021 at 10:53 AM Jarek Potiuk <ja...@potiuk.com <mailto:ja...@potiuk.com>> wrote:

It could be survivor bias, but at least in my memory pretty much every
 time I've seen something really serious, it's been also reported by
flake8 (which on its own is a combination of pep8, pyflakes + circular
 complexity).

 Let me explain a bit more context that built up in my mind over the
 last few months which made me change my mind.

 I think for such a tool, it is important to have as few
false-positives as possible. If you have a lot of false-positives, we
 are basically trading both - contributor's and reviewers time on
 analysing and explaining that "this is false positive" with the
 potential of introducing a mistake like a typo.

 As a reviewer, this is a recurring pattern recently that the
contributor complains about a "new" problem reported in the code they
 did not touch and then the reviewer has to get engaged and explain,
 and iterate often resulting in having to rebase the PR (sometimes
 again and again).

 When I look at our code:

 grep -R '# pylint: disable' | wc -l
 3370

 find . -name '*.py'  | wc -l
 4924

We have > 3000 false positives in our code and almost 5000 python files.

 Those false positives are time lost by both contributors and
reviewers. Those are the places where contributor and reviewer jointly decided that it's not worth to fix the problem reported by pylunt. Is
 3370/4920 a lot ? I think so.

 Yes it might mean we should disable some rules possibly. But when I
look at those rules to disable, those are the very ones that we would like to be able to discover. The list of those unique disables at the
 end of the mail.

 If you look at the pyflakes (which is one of the three tools in
 flake8) -> "Pyflakes makes a simple promise: it will never complain
 about style, and it will try very, very hard to never emit false
 positives." . I think it delivers.

 Pylint makes no such promise (and delivers as well).

 J.


 #### Currently disabled pylint rules:

grep -R '# pylint: disable' | sed 's/.*\(# pylint.*\)/\1/' | sort | uniq
 # pylint: disable=abstract-method
 # pylint: disable=assignment-from-no-return
 # pylint: disable=attribute-defined-outside-init
 # pylint: disable=broad-except
 # pylint: disable=C0103
 # pylint: disable=C0103, line-too-long
 # pylint: disable=C0111
 # pylint: disable=C0123
 # pylint: disable=C0301
 # pylint: disable=C0302
 # pylint: disable=c-extension-no-member
 # pylint: disable=c-extension-no-member, no-member
 # pylint: disable=c-extension-no-member,no-member
 # pylint: disable=comparison-with-callable
 # pylint: disable=comparison-with-itself
 # pylint: disable=consider-using-enumerate
 # pylint: disable=consider-using-generator
 # pylint: disable=consider-using-with
 # pylint: disable=consider-using-with,attribute-defined-outside-init
 # pylint: disable=cyclic-import
 # pylint: disable=dangerous-default-value
 # pylint: disable=E0012
 # pylint: disable=E1101
 # pylint: disable=E1102
 # pylint: disable=E1111
 # pylint: disable=expression-not-assigned
 # pylint: disable=global-statement
 # pylint: disable=global-statement,global-variable-not-assigned
 # pylint: disable=inconsistent-return-statements
 # pylint: disable=invalid-bool-returned, bad-option-value
 # pylint: disable=invalid-length-returned
 # pylint: disable=invalid-name
 # pylint: disable=invalid-name,missing-docstring
 # pylint: disable=invalid-name, unused-argument
 # pylint: disable=invalid-sequence-index
 # pylint: disable=invalid-str-returned
 # pylint: disable=len-as-condition
 # pylint: disable=line-too-long
 # pylint: disable=line-too-long # noqa
 # pylint: disable=logging-not-lazy
 # pylint: disable=lost-exception
 # pylint: disable=maybe-no-member
 # pylint: disable=missing-docstring
 # pylint: disable=missing-docstring, redefined-outer-name
 # pylint: disable=missing-function-docstring
 # pylint: disable=missing-kwoa
 # pylint: disable-msg=too-many-arguments
 # pylint: disable=no-else-break
 # pylint: disable=no-else-continue
 # pylint: disable=no-member
 # pylint: disable=no-member,invalid-name
 # pylint: disable=no-member,line-too-long
 # pylint: disable=no-member, maybe-no-member
 # pylint: disable=no-member # noqa
 # pylint: disable=no-method-argument
 # pylint: disable=no-name-in-module
 # pylint: disable=no-name-in-module,wrong-import-order
 # pylint: disable=non-parent-init-called
 # pylint: disable=no-self-argument
 # pylint: disable=not-a-mapping
 # pylint: disable=not-callable
 # pylint: disable=no-value-for-parameter
 # pylint: disable=pointless-statement
 # pylint: disable=protected-access
 # pylint: disable=protected-access,c-extension-no-member,no-member
 # pylint: disable=protected-access,no-member
 # pylint: disable=R0401
 # pylint: disable=R0902,R0904
 # pylint: disable=R0904, C0111
 # pylint: disable=R0904, C0111, C0302
 # pylint: disable=R0904, C0302
 # pylint: disable=R0913, C0302
 # pylint: disable=raising-format-tuple
 # pylint: disable=redefined-builtin
 # pylint: disable=redefined-builtin,unused-argument
# pylint: disable=redefined-builtin,unused-argument,too-many-arguments
 # pylint: disable=redefined-outer-name
 # pylint: disable=redefined-outer-name,reimported
 # pylint: disable=redefined-outer-name,reimported,unused-import
 # pylint: disable=redefined-outer-name,unused-argument
 # pylint: disable=reimported,unused-import,redefined-outer-name
 # pylint: disable=self-assigning-variable
 # pylint: disable=signature-differs
 # pylint: disable=signature-differs   # noqa: D403
 # pylint: disable=singleton-comparison
 # pylint: disable=subprocess-popen-preexec-fn
 # pylint: disable=subprocess-popen-preexec-fn,consider-using-with
 # pylint: disable=subprocess-run-check
 # pylint: disable=super-init-not-called
 # pylint: disable=syntax-error
 # pylint: disable=too-few-public-methods
 # pylint: disable=too-many-ancestors
 # pylint: disable=too-many-arguments
 # pylint: disable=too-many-arguments,inconsistent-return-statements
 # pylint: disable=too-many-arguments, too-many-locals
 # pylint: disable=too-many-arguments,too-many-locals
# pylint: disable=too-many-arguments,too-many-locals,too-many-branches # pylint: disable=too-many-arguments,too-many-locals, too-many-statements
 # pylint: disable=too-many-boolean-expressions
 # pylint: disable=too-many-branches
 # pylint: disable=too-many-function-args
 # pylint: disable=too-many-instance-attributes
 # pylint: disable=too-many-instance-attributes, too-many-arguments
# pylint: disable=too-many-instance-attributes,too-many-arguments,too-many-branches
 # pylint: disable=too-many-instance-attributes,too-many-locals
# pylint: disable=too-many-instance-attributes,too-many-public-methods
 # pylint: disable=too-many-lines
 # pylint: disable=too-many-locals
 # pylint: disable=too-many-locals,too-many-arguments
 # pylint: disable=too-many-locals,too-many-arguments,invalid-name
# pylint: disable=too-many-locals,too-many-arguments, too-many-branches
 # pylint: disable=too-many-locals,too-many-statements
 # pylint: disable=too-many-nested-blocks
 # pylint: disable=too-many-public-methods
 # pylint: disable=too-many-return-statements
 # pylint: disable=too-many-statements
 # pylint: disable=undefined-variable
 # pylint: disable=unexpected-keyword-arg
 # pylint: disable=ungrouped-imports
 # pylint: disable=unidiomatic-typecheck
 # pylint: disable=unnecessary-lambda
 # pylint: disable=unsubscriptable-object
 # pylint: disable=unsupported-membership-test
 # pylint: disable = unused-argument
 # pylint: disable=unused-argument
 # pylint: disable=unused-argument,invalid-name
 # pylint: disable=unused-argument,line-too-long
 # pylint: disable=unused-argument # pragma: no cover
 # pylint: disable=unused-argument,too-many-arguments,too-many-locals
 # pylint: disable=unused-argument, unused-variable
 # pylint: disable=unused-import
 # pylint: disable=unused-import,line-too-long
 # pylint: disable=unused-import # noqa
 # pylint: disable=unused-import  # noqa: F401
 # pylint: disable=unused-import # noqa: F401
 # pylint: disable=unused-variable
 # pylint: disable=unused-variable  # noqa
 # pylint: disable=using-constant-test
 # pylint: disable=W0104
 # pylint: disable=W0106
 # pylint: disable=W0108
 # pylint: disable=W0143
 # pylint: disable=W0201
 # pylint: disable=W0212
 # pylint: disable=W0612
 # pylint: disable=W0703
 # pylint: disable=wildcard-import
 # pylint: disable=wrong-import-order
 # pylint: disable=wrong-import-position


On Thu, Jun 24, 2021 at 1:57 AM Tomasz Urbaszek <turbas...@apache.org <mailto:turbas...@apache.org>> wrote:
 >
> -0.5 from me. Black and flake8 does not guarantee the same quality and consistency of code as pylint (for example using if list instead of if len(list) > 0 ).
 >
> > I can't remember the last time where pylint prevented any real error
 >
> Well, isn't it a survivor bias (the one usually represented by this picture of airplane)?
 >
> Pylint is slow and sometimes brittle but I think it may help. What I would consider is to keep pylint for providers - this code in particular is created and changed by plenty of people with different backgrounds and standards.
 >
 > Tomek
 >
> On Wed, 23 Jun 2021 at 21:45, Jarek Potiuk <ja...@potiuk.com <mailto:ja...@potiuk.com>> wrote:
 >>
>> Just to be precise - the vote will end on Monday 28th at 8.30 PM CEST
 >> (to account for weekend) ...
 >>
 >> J.
 >>
>> On Wed, Jun 23, 2021 at 9:29 PM Kaxil Naik <kaxiln...@gmail.com <mailto:kaxiln...@gmail.com>> wrote:
 >> >
>> > +1 -- Pylint is proving to be a headache and is very very slow locally this days
 >> >
>> > On Wed, Jun 23, 2021 at 8:02 PM Jarek Potiuk <ja...@potiuk.com <mailto:ja...@potiuk.com>> wrote:
 >> >>
 >> >> Why not +10000 Ash :) ?
 >> >>
 >> >> On Wed, Jun 23, 2021 at 9:00 PM Andrew Godwin
>> >> <andrew.god...@astronomer.io.invalid <mailto:andrew.god...@astronomer.io.invalid>> wrote:
 >> >> >
>> >> > I can't cast a binding vote on this, but I would also like to chime in with support for this move. We have plenty of protection from everything else, and while we could try to go through pylint and disable every superfluous check, I'm not sure it's worth it.
 >> >> >
 >> >> > Andrew
 >> >> >
>> >> > On Wed, Jun 23, 2021 at 12:49 PM Ash Berlin-Taylor <a...@apache.org <mailto:a...@apache.org>> wrote:
 >> >> >>
 >> >> >> Clearly I'm +7000 on this.
 >> >> >>
>> >> >> On 23 June 2021 19:38:13 BST, Xinbin Huang <bin.huan...@gmail.com <mailto:bin.huan...@gmail.com>> wrote:
 >> >> >>>
>> >> >>> I would like to deprecate it too, so count +1 from me. One question I have is: do we have any ways to detect and prevent cyclic imports?
 >> >> >>>
>> >> >>> On Wed, Jun 23, 2021 at 11:30 AM Jarek Potiuk <ja...@potiuk.com <mailto:ja...@potiuk.com>> wrote:
 >> >> >>>>
>> >> >>>> I think this subject has been raised a few times (last time by Ash).
 >> >> >>>> Finally I grew up to embrace it as well.
 >> >> >>>>
>> >> >>>> I think I am also fed-up by random pylint errors. Last time >> >> >>>> <https://github.com/apache/airflow/pull/15634/checks?check_run_id=2896761068>
 >> >> >>>>
>> >> >>>> We have many, many pylint exceptions all over our code. I can't >> >> >>>> remember the last time where pylint prevented any real error. As Ash >> >> >>>> (rightfully) mentioned in latest discussion on slack - we have >> >> >>>> mypy/flake/isort/black which report (and fix) vast majority of things
 >> >> >>>> pylint reports.
 >> >> >>>>
 >> >> >>>> I think this last error was the final drop for me.
 >> >> >>>>
 >> >> >>>> Should we remove pylint ?
 >> >> >>>>
 >> >> >>>> Consider it +1 from my side.
 >> >> >>>>
 >> >> >>>> J .
 >> >> >>>>
 >> >> >>>>
 >> >> >>>> --
 >> >> >>>> +48 660 796 129
 >> >>
 >> >>
 >> >>
 >> >> --
 >> >> +48 660 796 129
 >>
 >>
 >>
 >> --
 >> +48 660 796 129



 --
 +48 660 796 129



--
+48 660 796 129

Reply via email to