[VOTE] Disable pylint ?

[Jarek Potiuk <ja...@potiuk.com>]

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

Re: [VOTE] Disable pylint ?

[Jarek Potiuk <ja...@potiuk.com>]

I think I will not wait another hour :). > 72 hours passed already and
again few more people complained, so I better proceed.

Vote results:

* Jarek Potiuk +1 (binding)
* Xinbing Huang +1 (binding)
* Ash Berlin-Taylor ++1 (binding) - (I rounded the +7000 to the
nearest possible voting value which is ++1 "++1: 'Wow! I like this!
Let's do it!'" ;))
* Kaxil Naik +1 (binding)
* Tzu-ping Chung  +1 (binding now)
* Tomasz Urbaszek -0.5 (non-binding)
* Kamil Breguła +1 (binding)
* Andrew Goodwin +1 (non-binding)
* Brent Bovenzi +1 (non-binding)

The result of the vote is "passed". The result is +6 vs. -0.5  - in
favour of removing Pylint.

I proceed with https://github.com/apache/airflow/pull/16682 to remove
Pylint from our toolchain.

J.

On Mon, Jun 28, 2021 at 7:23 PM Ash Berlin-Taylor <as...@apache.org> wrote:
>
> Jarek had opened a PR https://github.com/apache/airflow/pull/16682
>
> On 28 June 2021 18:05:11 BST, Brent Bovenzi <br...@astronomer.io.INVALID> wrote:
>>
>> + 1 (non-binding)
>> It's quite annoying to get pylint errors when I haven't even touched python code in a PR.
>>
>> On Sat, Jun 26, 2021 at 2:50 PM Jarek Potiuk <ja...@potiuk.com> wrote:
>>>
>>> One more comment. Since the start of that vote (3 days ago) I think I
>>> have taken part in ~10 PRs (I think ~4 of them were mine) where
>>> suddenly new pylint errors appeared like a rabbit from a magician hat.
>>>
>>> Some quotes from contributors:
>>>
>>> "@kaxil can you help me with this? Not able to get this error."
>>>
>>> "Hey @mik-laj
>>> CI on this PR is failing due to unrelated issues.
>>> Have tagged you for review, if you like.
>>> Otherwise I'm happy to wait with this one until the CI issues get
>>> resolved, and retry it later. Have tried a few times though".
>>>
>>> I think that clearly shows the overhead pylint creates in our process
>>> - for both: contributors and committers.
>>>
>>> J.
>>>
>>>
>>>
>>>
>>> J.
>>>
>>>
>>> On Sat, Jun 26, 2021 at 4:30 PM .... <dz...@gmail.com> wrote:
>>> >
>>> > +1 Most of the time I disabled it locally and only ran it on CI.
>>> >
>>> > czw., 24 cze 2021 o 20:20 Tomasz Urbaszek <tu...@apache.org> napisał(a):
>>> > >
>>> > > While I fully agree with all the points about pylint being slow and cumbersome I still keep my -0.5, which if I correctly understand is not blocking but is just expression of my opinion :D
>>> > >
>>> > > Thanks,
>>> > > Tomek
>>> > >
>>> > > On Thu, 24 Jun 2021 at 13:07, Jarek Potiuk <ja...@potiuk.com> wrote:
>>> > >>
>>> > >> Just to give an example of the problems we have to deal with re: false
>>> > >> positives.
>>> > >>
>>> > >> I ALMOST had a working Python 3.9 change this morning. ALMOST because
>>> > >> when I run it everything worked except about hundred new 'no-member'
>>> > >> errors in a file that I have
>>> > >> not touched (and it has no relation whatsoever with the changes I made).
>>> > >>
>>> > >> No new pylint was released. Just other, unrelated code was changed:
>>> > >>
>>> > >> https://github.com/potiuk/airflow/runs/2903575190?check_suite_focus=true#step:9:48
>>> > >>
>>> > >> This is something that recently started to occur almost daily - for
>>> > >> various people. This is mightily annoying and causes even more delays
>>> > >> and reruns of the builds (thus money) for really bogus reasons.
>>> > >>
>>> > >> J.
>>> > >>
>>> > >> On Thu, Jun 24, 2021 at 11:33 AM Ash Berlin-Taylor <as...@apache.org> wrote:
>>> > >> >
>>> > >> > *23m23s on our more-powerfull self-hosted runners.
>>> > >> >
>>> > >> > On Thu, Jun 24 2021 at 10:32:43 +0100, Ash Berlin-Taylor <as...@apache.org> wrote:
>>> > >> >
>>> > >> > The biggest advantage to me: reducing test time on CI and pre-commit.
>>> > >> >
>>> > >> > Pylint is slow.
>>> > >> >
>>> > >> > Oh my god, I just checked and it's even slower than I realised
>>> > >> >
>>> > >> > 46m25s on Github Runners, 23m23s for the "pylint" job.
>>> > >> >
>>> > >> > https://github.com/apache/airflow/runs/2902755059?check_suite_focus=true
>>> > >> > https://github.com/apache/airflow/runs/2900159473?check_suite_focus=true
>>> > >> >
>>> > >> > There is no way pylint is worth that much test time!
>>> > >> >
>>> > >> > -ash
>>> > >> >
>>> > >> > On Thu, Jun 24 2021 at 11:22:53 +0200, Jarek Potiuk <ja...@potiuk.com> wrote:
>>> > >> >
>>> > >> > Good point Ash - I think we could indeed (if we decide to disable pylint) do this very thing indeed - identify the "important" aspects and try to find a plugin in flake8 for it. With our codebase, I think it will be rather easy to find if it works well for us (and we could do it as part of the `pylint-disable` PR and iterate until we are happy). I think it would streamline the development of ours a bit. J. On Thu, Jun 24, 2021 at 11:12 AM Ash Berlin-Taylor <as...@apache.org> wrote:
>>> > >> >
>>> > >> > 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> 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 <tu...@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> 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 <ka...@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> wrote: >> >> >> >> Why not +10000 Ash :) ? >> >> >> >> On Wed, Jun 23, 2021 at 9:00 PM Andrew Godwin >> >> <an...@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 <as...@apache.org> wrote: >> >> >> >> >> >> Clearly I'm +7000 on this. >> >> >> >> >> >> On 23 June 2021 19:38:13 BST, Xinbin Huang <bi...@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> 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
>>> > >> >
>>> > >> > --
>>> > >> > +48 660 796 129
>>> > >>
>>> > >>
>>> > >>
>>> > >> --
>>> > >> +48 660 796 129
>>>
>>>
>>>
>>> --
>>> +48 660 796 129



-- 
+48 660 796 129

Re: [VOTE] Disable pylint ?

[Ash Berlin-Taylor <as...@apache.org>]

Jarek had opened a PR https://github.com/apache/airflow/pull/16682

On 28 June 2021 18:05:11 BST, Brent Bovenzi <br...@astronomer.io.INVALID> wrote:
>+ 1 (non-binding)
>It's quite annoying to get pylint errors when I haven't even touched python
>code in a PR.
>
>On Sat, Jun 26, 2021 at 2:50 PM Jarek Potiuk <ja...@potiuk.com> wrote:
>
>> One more comment. Since the start of that vote (3 days ago) I think I
>> have taken part in ~10 PRs (I think ~4 of them were mine) where
>> suddenly new pylint errors appeared like a rabbit from a magician hat.
>>
>> Some quotes from contributors:
>>
>> "@kaxil can you help me with this? Not able to get this error."
>>
>> "Hey @mik-laj
>> CI on this PR is failing due to unrelated issues.
>> Have tagged you for review, if you like.
>> Otherwise I'm happy to wait with this one until the CI issues get
>> resolved, and retry it later. Have tried a few times though".
>>
>> I think that clearly shows the overhead pylint creates in our process
>> - for both: contributors and committers.
>>
>> J.
>>
>>
>>
>>
>> J.
>>
>>
>> On Sat, Jun 26, 2021 at 4:30 PM .... <dz...@gmail.com> wrote:
>> >
>> > +1 Most of the time I disabled it locally and only ran it on CI.
>> >
>> > czw., 24 cze 2021 o 20:20 Tomasz Urbaszek <tu...@apache.org>
>> napisał(a):
>> > >
>> > > While I fully agree with all the points about pylint being slow and
>> cumbersome I still keep my -0.5, which if I correctly understand is not
>> blocking but is just expression of my opinion :D
>> > >
>> > > Thanks,
>> > > Tomek
>> > >
>> > > On Thu, 24 Jun 2021 at 13:07, Jarek Potiuk <ja...@potiuk.com> wrote:
>> > >>
>> > >> Just to give an example of the problems we have to deal with re: false
>> > >> positives.
>> > >>
>> > >> I ALMOST had a working Python 3.9 change this morning. ALMOST because
>> > >> when I run it everything worked except about hundred new 'no-member'
>> > >> errors in a file that I have
>> > >> not touched (and it has no relation whatsoever with the changes I
>> made).
>> > >>
>> > >> No new pylint was released. Just other, unrelated code was changed:
>> > >>
>> > >>
>> https://github.com/potiuk/airflow/runs/2903575190?check_suite_focus=true#step:9:48
>> > >>
>> > >> This is something that recently started to occur almost daily - for
>> > >> various people. This is mightily annoying and causes even more delays
>> > >> and reruns of the builds (thus money) for really bogus reasons.
>> > >>
>> > >> J.
>> > >>
>> > >> On Thu, Jun 24, 2021 at 11:33 AM Ash Berlin-Taylor <as...@apache.org>
>> wrote:
>> > >> >
>> > >> > *23m23s on our more-powerfull self-hosted runners.
>> > >> >
>> > >> > On Thu, Jun 24 2021 at 10:32:43 +0100, Ash Berlin-Taylor <
>> ash@apache.org> wrote:
>> > >> >
>> > >> > The biggest advantage to me: reducing test time on CI and
>> pre-commit.
>> > >> >
>> > >> > Pylint is slow.
>> > >> >
>> > >> > Oh my god, I just checked and it's even slower than I realised
>> > >> >
>> > >> > 46m25s on Github Runners, 23m23s for the "pylint" job.
>> > >> >
>> > >> >
>> https://github.com/apache/airflow/runs/2902755059?check_suite_focus=true
>> > >> >
>> https://github.com/apache/airflow/runs/2900159473?check_suite_focus=true
>> > >> >
>> > >> > There is no way pylint is worth that much test time!
>> > >> >
>> > >> > -ash
>> > >> >
>> > >> > On Thu, Jun 24 2021 at 11:22:53 +0200, Jarek Potiuk <
>> jarek@potiuk.com> wrote:
>> > >> >
>> > >> > Good point Ash - I think we could indeed (if we decide to disable
>> pylint) do this very thing indeed - identify the "important" aspects and
>> try to find a plugin in flake8 for it. With our codebase, I think it will
>> be rather easy to find if it works well for us (and we could do it as part
>> of the `pylint-disable` PR and iterate until we are happy). I think it
>> would streamline the development of ours a bit. J. On Thu, Jun 24, 2021 at
>> 11:12 AM Ash Berlin-Taylor <as...@apache.org> wrote:
>> > >> >
>> > >> > 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> 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 <tu...@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> 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 <
>> kaxilnaik@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> wrote: >> >> >> >> Why
>> not +10000 Ash :) ? >> >> >> >> On Wed, Jun 23, 2021 at 9:00 PM Andrew
>> Godwin >> >> <an...@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 <as...@apache.org>
>> wrote: >> >> >> >> >> >> Clearly I'm +7000 on this. >> >> >> >> >> >> On 23
>> June 2021 19:38:13 BST, Xinbin Huang <bi...@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 <
>> jarek@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
>> > >> >
>> > >> > --
>> > >> > +48 660 796 129
>> > >>
>> > >>
>> > >>
>> > >> --
>> > >> +48 660 796 129
>>
>>
>>
>> --
>> +48 660 796 129
>>

Re: [VOTE] Disable pylint ?

[Brent Bovenzi <br...@astronomer.io.INVALID>]

+ 1 (non-binding)
It's quite annoying to get pylint errors when I haven't even touched python
code in a PR.

On Sat, Jun 26, 2021 at 2:50 PM Jarek Potiuk <ja...@potiuk.com> wrote:

> One more comment. Since the start of that vote (3 days ago) I think I
> have taken part in ~10 PRs (I think ~4 of them were mine) where
> suddenly new pylint errors appeared like a rabbit from a magician hat.
>
> Some quotes from contributors:
>
> "@kaxil can you help me with this? Not able to get this error."
>
> "Hey @mik-laj
> CI on this PR is failing due to unrelated issues.
> Have tagged you for review, if you like.
> Otherwise I'm happy to wait with this one until the CI issues get
> resolved, and retry it later. Have tried a few times though".
>
> I think that clearly shows the overhead pylint creates in our process
> - for both: contributors and committers.
>
> J.
>
>
>
>
> J.
>
>
> On Sat, Jun 26, 2021 at 4:30 PM .... <dz...@gmail.com> wrote:
> >
> > +1 Most of the time I disabled it locally and only ran it on CI.
> >
> > czw., 24 cze 2021 o 20:20 Tomasz Urbaszek <tu...@apache.org>
> napisał(a):
> > >
> > > While I fully agree with all the points about pylint being slow and
> cumbersome I still keep my -0.5, which if I correctly understand is not
> blocking but is just expression of my opinion :D
> > >
> > > Thanks,
> > > Tomek
> > >
> > > On Thu, 24 Jun 2021 at 13:07, Jarek Potiuk <ja...@potiuk.com> wrote:
> > >>
> > >> Just to give an example of the problems we have to deal with re: false
> > >> positives.
> > >>
> > >> I ALMOST had a working Python 3.9 change this morning. ALMOST because
> > >> when I run it everything worked except about hundred new 'no-member'
> > >> errors in a file that I have
> > >> not touched (and it has no relation whatsoever with the changes I
> made).
> > >>
> > >> No new pylint was released. Just other, unrelated code was changed:
> > >>
> > >>
> https://github.com/potiuk/airflow/runs/2903575190?check_suite_focus=true#step:9:48
> > >>
> > >> This is something that recently started to occur almost daily - for
> > >> various people. This is mightily annoying and causes even more delays
> > >> and reruns of the builds (thus money) for really bogus reasons.
> > >>
> > >> J.
> > >>
> > >> On Thu, Jun 24, 2021 at 11:33 AM Ash Berlin-Taylor <as...@apache.org>
> wrote:
> > >> >
> > >> > *23m23s on our more-powerfull self-hosted runners.
> > >> >
> > >> > On Thu, Jun 24 2021 at 10:32:43 +0100, Ash Berlin-Taylor <
> ash@apache.org> wrote:
> > >> >
> > >> > The biggest advantage to me: reducing test time on CI and
> pre-commit.
> > >> >
> > >> > Pylint is slow.
> > >> >
> > >> > Oh my god, I just checked and it's even slower than I realised
> > >> >
> > >> > 46m25s on Github Runners, 23m23s for the "pylint" job.
> > >> >
> > >> >
> https://github.com/apache/airflow/runs/2902755059?check_suite_focus=true
> > >> >
> https://github.com/apache/airflow/runs/2900159473?check_suite_focus=true
> > >> >
> > >> > There is no way pylint is worth that much test time!
> > >> >
> > >> > -ash
> > >> >
> > >> > On Thu, Jun 24 2021 at 11:22:53 +0200, Jarek Potiuk <
> jarek@potiuk.com> wrote:
> > >> >
> > >> > Good point Ash - I think we could indeed (if we decide to disable
> pylint) do this very thing indeed - identify the "important" aspects and
> try to find a plugin in flake8 for it. With our codebase, I think it will
> be rather easy to find if it works well for us (and we could do it as part
> of the `pylint-disable` PR and iterate until we are happy). I think it
> would streamline the development of ours a bit. J. On Thu, Jun 24, 2021 at
> 11:12 AM Ash Berlin-Taylor <as...@apache.org> wrote:
> > >> >
> > >> > 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> 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 <tu...@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> 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 <
> kaxilnaik@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> wrote: >> >> >> >> Why
> not +10000 Ash :) ? >> >> >> >> On Wed, Jun 23, 2021 at 9:00 PM Andrew
> Godwin >> >> <an...@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 <as...@apache.org>
> wrote: >> >> >> >> >> >> Clearly I'm +7000 on this. >> >> >> >> >> >> On 23
> June 2021 19:38:13 BST, Xinbin Huang <bi...@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 <
> jarek@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
> > >> >
> > >> > --
> > >> > +48 660 796 129
> > >>
> > >>
> > >>
> > >> --
> > >> +48 660 796 129
>
>
>
> --
> +48 660 796 129
>

Re: [VOTE] Disable pylint ?

[Jarek Potiuk <ja...@potiuk.com>]

One more comment. Since the start of that vote (3 days ago) I think I
have taken part in ~10 PRs (I think ~4 of them were mine) where
suddenly new pylint errors appeared like a rabbit from a magician hat.

Some quotes from contributors:

"@kaxil can you help me with this? Not able to get this error."

"Hey @mik-laj
CI on this PR is failing due to unrelated issues.
Have tagged you for review, if you like.
Otherwise I'm happy to wait with this one until the CI issues get
resolved, and retry it later. Have tried a few times though".

I think that clearly shows the overhead pylint creates in our process
- for both: contributors and committers.

J.




J.


On Sat, Jun 26, 2021 at 4:30 PM .... <dz...@gmail.com> wrote:
>
> +1 Most of the time I disabled it locally and only ran it on CI.
>
> czw., 24 cze 2021 o 20:20 Tomasz Urbaszek <tu...@apache.org> napisał(a):
> >
> > While I fully agree with all the points about pylint being slow and cumbersome I still keep my -0.5, which if I correctly understand is not blocking but is just expression of my opinion :D
> >
> > Thanks,
> > Tomek
> >
> > On Thu, 24 Jun 2021 at 13:07, Jarek Potiuk <ja...@potiuk.com> wrote:
> >>
> >> Just to give an example of the problems we have to deal with re: false
> >> positives.
> >>
> >> I ALMOST had a working Python 3.9 change this morning. ALMOST because
> >> when I run it everything worked except about hundred new 'no-member'
> >> errors in a file that I have
> >> not touched (and it has no relation whatsoever with the changes I made).
> >>
> >> No new pylint was released. Just other, unrelated code was changed:
> >>
> >> https://github.com/potiuk/airflow/runs/2903575190?check_suite_focus=true#step:9:48
> >>
> >> This is something that recently started to occur almost daily - for
> >> various people. This is mightily annoying and causes even more delays
> >> and reruns of the builds (thus money) for really bogus reasons.
> >>
> >> J.
> >>
> >> On Thu, Jun 24, 2021 at 11:33 AM Ash Berlin-Taylor <as...@apache.org> wrote:
> >> >
> >> > *23m23s on our more-powerfull self-hosted runners.
> >> >
> >> > On Thu, Jun 24 2021 at 10:32:43 +0100, Ash Berlin-Taylor <as...@apache.org> wrote:
> >> >
> >> > The biggest advantage to me: reducing test time on CI and pre-commit.
> >> >
> >> > Pylint is slow.
> >> >
> >> > Oh my god, I just checked and it's even slower than I realised
> >> >
> >> > 46m25s on Github Runners, 23m23s for the "pylint" job.
> >> >
> >> > https://github.com/apache/airflow/runs/2902755059?check_suite_focus=true
> >> > https://github.com/apache/airflow/runs/2900159473?check_suite_focus=true
> >> >
> >> > There is no way pylint is worth that much test time!
> >> >
> >> > -ash
> >> >
> >> > On Thu, Jun 24 2021 at 11:22:53 +0200, Jarek Potiuk <ja...@potiuk.com> wrote:
> >> >
> >> > Good point Ash - I think we could indeed (if we decide to disable pylint) do this very thing indeed - identify the "important" aspects and try to find a plugin in flake8 for it. With our codebase, I think it will be rather easy to find if it works well for us (and we could do it as part of the `pylint-disable` PR and iterate until we are happy). I think it would streamline the development of ours a bit. J. On Thu, Jun 24, 2021 at 11:12 AM Ash Berlin-Taylor <as...@apache.org> wrote:
> >> >
> >> > 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> 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 <tu...@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> 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 <ka...@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> wrote: >> >> >> >> Why not +10000 Ash :) ? >> >> >> >> On Wed, Jun 23, 2021 at 9:00 PM Andrew Godwin >> >> <an...@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 <as...@apache.org> wrote: >> >> >> >> >> >> Clearly I'm +7000 on this. >> >> >> >> >> >> On 23 June 2021 19:38:13 BST, Xinbin Huang <bi...@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> 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
> >> >
> >> > --
> >> > +48 660 796 129
> >>
> >>
> >>
> >> --
> >> +48 660 796 129



--
+48 660 796 129

Re: [VOTE] Disable pylint ?

["...." <dz...@gmail.com>]

+1 Most of the time I disabled it locally and only ran it on CI.

czw., 24 cze 2021 o 20:20 Tomasz Urbaszek <tu...@apache.org> napisał(a):
>
> While I fully agree with all the points about pylint being slow and cumbersome I still keep my -0.5, which if I correctly understand is not blocking but is just expression of my opinion :D
>
> Thanks,
> Tomek
>
> On Thu, 24 Jun 2021 at 13:07, Jarek Potiuk <ja...@potiuk.com> wrote:
>>
>> Just to give an example of the problems we have to deal with re: false
>> positives.
>>
>> I ALMOST had a working Python 3.9 change this morning. ALMOST because
>> when I run it everything worked except about hundred new 'no-member'
>> errors in a file that I have
>> not touched (and it has no relation whatsoever with the changes I made).
>>
>> No new pylint was released. Just other, unrelated code was changed:
>>
>> https://github.com/potiuk/airflow/runs/2903575190?check_suite_focus=true#step:9:48
>>
>> This is something that recently started to occur almost daily - for
>> various people. This is mightily annoying and causes even more delays
>> and reruns of the builds (thus money) for really bogus reasons.
>>
>> J.
>>
>> On Thu, Jun 24, 2021 at 11:33 AM Ash Berlin-Taylor <as...@apache.org> wrote:
>> >
>> > *23m23s on our more-powerfull self-hosted runners.
>> >
>> > On Thu, Jun 24 2021 at 10:32:43 +0100, Ash Berlin-Taylor <as...@apache.org> wrote:
>> >
>> > The biggest advantage to me: reducing test time on CI and pre-commit.
>> >
>> > Pylint is slow.
>> >
>> > Oh my god, I just checked and it's even slower than I realised
>> >
>> > 46m25s on Github Runners, 23m23s for the "pylint" job.
>> >
>> > https://github.com/apache/airflow/runs/2902755059?check_suite_focus=true
>> > https://github.com/apache/airflow/runs/2900159473?check_suite_focus=true
>> >
>> > There is no way pylint is worth that much test time!
>> >
>> > -ash
>> >
>> > On Thu, Jun 24 2021 at 11:22:53 +0200, Jarek Potiuk <ja...@potiuk.com> wrote:
>> >
>> > Good point Ash - I think we could indeed (if we decide to disable pylint) do this very thing indeed - identify the "important" aspects and try to find a plugin in flake8 for it. With our codebase, I think it will be rather easy to find if it works well for us (and we could do it as part of the `pylint-disable` PR and iterate until we are happy). I think it would streamline the development of ours a bit. J. On Thu, Jun 24, 2021 at 11:12 AM Ash Berlin-Taylor <as...@apache.org> wrote:
>> >
>> > 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> 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 <tu...@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> 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 <ka...@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> wrote: >> >> >> >> Why not +10000 Ash :) ? >> >> >> >> On Wed, Jun 23, 2021 at 9:00 PM Andrew Godwin >> >> <an...@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 <as...@apache.org> wrote: >> >> >> >> >> >> Clearly I'm +7000 on this. >> >> >> >> >> >> On 23 June 2021 19:38:13 BST, Xinbin Huang <bi...@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> 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
>> >
>> > --
>> > +48 660 796 129
>>
>>
>>
>> --
>> +48 660 796 129

Re: [VOTE] Disable pylint ?

[Tomasz Urbaszek <tu...@apache.org>]

While I fully agree with all the points about pylint being slow and
cumbersome I still keep my -0.5, which if I correctly understand is not
blocking but is just expression of my opinion :D

Thanks,
Tomek

On Thu, 24 Jun 2021 at 13:07, Jarek Potiuk <ja...@potiuk.com> wrote:

> Just to give an example of the problems we have to deal with re: false
> positives.
>
> I ALMOST had a working Python 3.9 change this morning. ALMOST because
> when I run it everything worked except about hundred new 'no-member'
> errors in a file that I have
> not touched (and it has no relation whatsoever with the changes I made).
>
> No new pylint was released. Just other, unrelated code was changed:
>
>
> https://github.com/potiuk/airflow/runs/2903575190?check_suite_focus=true#step:9:48
>
> This is something that recently started to occur almost daily - for
> various people. This is mightily annoying and causes even more delays
> and reruns of the builds (thus money) for really bogus reasons.
>
> J.
>
> On Thu, Jun 24, 2021 at 11:33 AM Ash Berlin-Taylor <as...@apache.org> wrote:
> >
> > *23m23s on our more-powerfull self-hosted runners.
> >
> > On Thu, Jun 24 2021 at 10:32:43 +0100, Ash Berlin-Taylor <as...@apache.org>
> wrote:
> >
> > The biggest advantage to me: reducing test time on CI and pre-commit.
> >
> > Pylint is slow.
> >
> > Oh my god, I just checked and it's even slower than I realised
> >
> > 46m25s on Github Runners, 23m23s for the "pylint" job.
> >
> > https://github.com/apache/airflow/runs/2902755059?check_suite_focus=true
> > https://github.com/apache/airflow/runs/2900159473?check_suite_focus=true
> >
> > There is no way pylint is worth that much test time!
> >
> > -ash
> >
> > On Thu, Jun 24 2021 at 11:22:53 +0200, Jarek Potiuk <ja...@potiuk.com>
> wrote:
> >
> > Good point Ash - I think we could indeed (if we decide to disable
> pylint) do this very thing indeed - identify the "important" aspects and
> try to find a plugin in flake8 for it. With our codebase, I think it will
> be rather easy to find if it works well for us (and we could do it as part
> of the `pylint-disable` PR and iterate until we are happy). I think it
> would streamline the development of ours a bit. J. On Thu, Jun 24, 2021 at
> 11:12 AM Ash Berlin-Taylor <as...@apache.org> wrote:
> >
> > 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> 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 <tu...@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> 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 <
> kaxilnaik@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> wrote: >> >> >> >> Why
> not +10000 Ash :) ? >> >> >> >> On Wed, Jun 23, 2021 at 9:00 PM Andrew
> Godwin >> >> <an...@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 <as...@apache.org>
> wrote: >> >> >> >> >> >> Clearly I'm +7000 on this. >> >> >> >> >> >> On 23
> June 2021 19:38:13 BST, Xinbin Huang <bi...@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 <
> jarek@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
> >
> > --
> > +48 660 796 129
>
>
>
> --
> +48 660 796 129
>

Re: [VOTE] Disable pylint ?

[Tzu-ping Chung <tp...@astronomer.io.INVALID>]

Pylint has always been a mix-bag to me. There are a lot of good checks 
that help prevent intermediate level errors, but there are also too many 
opinionated checks enforcing superfluous metrics (some I’d even say 
wrong, e.g. too-few-public-methods and too-many-return-statements).

Airflow’s Python code (more specifically SQLAlchemy’s query DSL) makes 
Pylint especially confused since it doesn’t (can’t?) distinguish cases 
like `query.filter(DagRun.external_trigger == False` to `if allowed == 
False:`. Yes, you can tell Pylint to ignore things, and explicit is 
better than implicit, but Airflow’s code base right now has so many of 
those it is at the point that developers are almost writing code to 
satisfy Pylint rather for humans to read. The goal of tools is to help 
people, and Pylint is hurting more than helping me, personally.

FWIW, most (all? not sure) of the Python projects I’m involved with 
don’t use Pylint, and they still maintain good code quality fine. Very 
few open source contributors actually make mistakes like `if allowed == 
False` in the first place, and the manual conversation needed to correct 
them is very minimal when it occasionally happens. And Pylint don’t 
actually prevent that round of review for Airflow, but simply turning it 
from “hey this line is not idiomatic Python can you fix that” to “hey 
you need to do this to make Pylint happy or we can’t merge”.

So overall I am also +1 to removing Pylint. Or, if we must keep it, I 
would strongly encourage disabling checks that we have too many in-line 
exceptions for (e.g. the two mentioned above, comparison-with-callable, 
comparison-with-singleton, too-many-public-methods, the list goes on). 
And yes, that means we need to manually review those `if allowed == 
False` mistakes, but that sacrifice is well worth the benefits IMO.

Re: [VOTE] Disable pylint ?

[Jarek Potiuk <ja...@potiuk.com>]

Just to give an example of the problems we have to deal with re: false
positives.

I ALMOST had a working Python 3.9 change this morning. ALMOST because
when I run it everything worked except about hundred new 'no-member'
errors in a file that I have
not touched (and it has no relation whatsoever with the changes I made).

No new pylint was released. Just other, unrelated code was changed:

https://github.com/potiuk/airflow/runs/2903575190?check_suite_focus=true#step:9:48

This is something that recently started to occur almost daily - for
various people. This is mightily annoying and causes even more delays
and reruns of the builds (thus money) for really bogus reasons.

J.

On Thu, Jun 24, 2021 at 11:33 AM Ash Berlin-Taylor <as...@apache.org> wrote:
>
> *23m23s on our more-powerfull self-hosted runners.
>
> On Thu, Jun 24 2021 at 10:32:43 +0100, Ash Berlin-Taylor <as...@apache.org> wrote:
>
> The biggest advantage to me: reducing test time on CI and pre-commit.
>
> Pylint is slow.
>
> Oh my god, I just checked and it's even slower than I realised
>
> 46m25s on Github Runners, 23m23s for the "pylint" job.
>
> https://github.com/apache/airflow/runs/2902755059?check_suite_focus=true
> https://github.com/apache/airflow/runs/2900159473?check_suite_focus=true
>
> There is no way pylint is worth that much test time!
>
> -ash
>
> On Thu, Jun 24 2021 at 11:22:53 +0200, Jarek Potiuk <ja...@potiuk.com> wrote:
>
> Good point Ash - I think we could indeed (if we decide to disable pylint) do this very thing indeed - identify the "important" aspects and try to find a plugin in flake8 for it. With our codebase, I think it will be rather easy to find if it works well for us (and we could do it as part of the `pylint-disable` PR and iterate until we are happy). I think it would streamline the development of ours a bit. J. On Thu, Jun 24, 2021 at 11:12 AM Ash Berlin-Taylor <as...@apache.org> wrote:
>
> 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> 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 <tu...@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> 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 <ka...@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> wrote: >> >> >> >> Why not +10000 Ash :) ? >> >> >> >> On Wed, Jun 23, 2021 at 9:00 PM Andrew Godwin >> >> <an...@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 <as...@apache.org> wrote: >> >> >> >> >> >> Clearly I'm +7000 on this. >> >> >> >> >> >> On 23 June 2021 19:38:13 BST, Xinbin Huang <bi...@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> 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
>
> --
> +48 660 796 129



-- 
+48 660 796 129

Re: [VOTE] Disable pylint ?

[Ash Berlin-Taylor <as...@apache.org>]

*23m23s on our more-powerfull self-hosted runners.

On Thu, Jun 24 2021 at 10:32:43 +0100, Ash Berlin-Taylor 
<as...@apache.org> wrote:
> The biggest advantage to me: reducing test time on CI and pre-commit.
> 
> Pylint is slow.
> 
> Oh my god, I just checked and it's even slower than I realised
> 
> 46m25s on Github Runners, 23m23s for the "pylint" job.
> 
> <https://github.com/apache/airflow/runs/2902755059?check_suite_focus=true>
> <https://github.com/apache/airflow/runs/2900159473?check_suite_focus=true>
> 
> There is no way pylint is worth that much test time!
> 
> -ash
> 
> On Thu, Jun 24 2021 at 11:22:53 +0200, Jarek Potiuk 
> <ja...@potiuk.com> wrote:
>> Good point Ash - I think we could indeed (if we decide to disable
>> pylint) do this very thing indeed - identify the "important" aspects
>> and try to find a plugin in flake8 for it.
>> With our codebase, I think it will be rather easy to find if it works
>> well for us (and we could do it as part of the `pylint-disable` PR 
>> and
>> iterate until we are happy).
>> 
>> I think it would streamline the development of ours a bit.
>> 
>> J.
>> 
>> On Thu, Jun 24, 2021 at 11:12 AM Ash Berlin-Taylor <ash@apache.org 
>> <ma...@apache.org>> wrote:
>>> 
>>>  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 
>>> <jarek@potiuk.com <ma...@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 <jarek@potiuk.com <ma...@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 <turbaszek@apache.org 
>>> <ma...@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 
>>> <jarek@potiuk.com <ma...@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 <kaxilnaik@gmail.com 
>>> <ma...@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 
>>> <jarek@potiuk.com <ma...@potiuk.com>> wrote: >> >> >> >> Why 
>>> not +10000 Ash :) ? >> >> >> >> On Wed, Jun 23, 2021 at 9:00 PM 
>>> Andrew Godwin >> >> <andrew.godwin@astronomer.io.invalid 
>>> <ma...@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 <ash@apache.org <ma...@apache.org>> 
>>> wrote: >> >> >> >> >> >> Clearly I'm +7000 on this. >> >> >> >> >> 
>>> >> On 23 June 2021 19:38:13 BST, Xinbin Huang 
>>> <bin.huangxb@gmail.com <ma...@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 <jarek@potiuk.com <ma...@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
>> 
>> 
>> 
>> --
>> +48 660 796 129

Re: [VOTE] Disable pylint ?

[Ash Berlin-Taylor <as...@apache.org>]

The biggest advantage to me: reducing test time on CI and pre-commit.

Pylint is slow.

Oh my god, I just checked and it's even slower than I realised

46m25s on Github Runners, 23m23s for the "pylint" job.

<https://github.com/apache/airflow/runs/2902755059?check_suite_focus=true>
<https://github.com/apache/airflow/runs/2900159473?check_suite_focus=true>

There is no way pylint is worth that much test time!

-ash

On Thu, Jun 24 2021 at 11:22:53 +0200, Jarek Potiuk <ja...@potiuk.com> 
wrote:
> Good point Ash - I think we could indeed (if we decide to disable
> pylint) do this very thing indeed - identify the "important" aspects
> and try to find a plugin in flake8 for it.
> With our codebase, I think it will be rather easy to find if it works
> well for us (and we could do it as part of the `pylint-disable` PR and
> iterate until we are happy).
> 
> I think it would streamline the development of ours a bit.
> 
> J.
> 
> On Thu, Jun 24, 2021 at 11:12 AM Ash Berlin-Taylor <ash@apache.org 
> <ma...@apache.org>> wrote:
>> 
>>  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 
>> <jarek@potiuk.com <ma...@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 <jarek@potiuk.com 
>> <ma...@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 
>> <turbaszek@apache.org <ma...@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 <jarek@potiuk.com <ma...@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 <kaxilnaik@gmail.com 
>> <ma...@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 
>> <jarek@potiuk.com <ma...@potiuk.com>> wrote: >> >> >> >> Why 
>> not +10000 Ash :) ? >> >> >> >> On Wed, Jun 23, 2021 at 9:00 PM 
>> Andrew Godwin >> >> <andrew.godwin@astronomer.io.invalid 
>> <ma...@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 <ash@apache.org <ma...@apache.org>> wrote: >> >> 
>> >> >> >> >> Clearly I'm +7000 on this. >> >> >> >> >> >> On 23 June 
>> 2021 19:38:13 BST, Xinbin Huang <bin.huangxb@gmail.com 
>> <ma...@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 
>> <jarek@potiuk.com <ma...@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
> 
> 
> 
> --
> +48 660 796 129

Re: [VOTE] Disable pylint ?

[Jarek Potiuk <ja...@potiuk.com>]

Good point Ash - I think we could indeed (if we decide to disable
pylint) do this very thing indeed - identify the "important" aspects
and try to find a plugin in flake8 for it.
With our codebase, I think it will be rather easy to find if it works
well for us (and we could do it as part of the `pylint-disable` PR and
iterate until we are happy).

I think it would streamline the development of ours a bit.

J.

On Thu, Jun 24, 2021 at 11:12 AM Ash Berlin-Taylor <as...@apache.org> wrote:
>
> 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> 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 <tu...@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> 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 <ka...@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> wrote: >> >> >> >> Why not +10000 Ash :) ? >> >> >> >> On Wed, Jun 23, 2021 at 9:00 PM Andrew Godwin >> >> <an...@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 <as...@apache.org> wrote: >> >> >> >> >> >> Clearly I'm +7000 on this. >> >> >> >> >> >> On 23 June 2021 19:38:13 BST, Xinbin Huang <bi...@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> 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



-- 
+48 660 796 129

Re: [VOTE] Disable pylint ?

[Ash Berlin-Taylor <as...@apache.org>]

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 <jarek@potiuk.com 
> <ma...@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 
>> <turbaszek@apache.org <ma...@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 <jarek@potiuk.com 
>> <ma...@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 <kaxilnaik@gmail.com 
>> <ma...@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 <jarek@potiuk.com 
>> <ma...@potiuk.com>> wrote:
>>  >> >>
>>  >> >> Why not +10000 Ash :) ?
>>  >> >>
>>  >> >> On Wed, Jun 23, 2021 at 9:00 PM Andrew Godwin
>>  >> >> <andrew.godwin@astronomer.io.invalid 
>> <ma...@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 
>> <ash@apache.org <ma...@apache.org>> wrote:
>>  >> >> >>
>>  >> >> >> Clearly I'm +7000 on this.
>>  >> >> >>
>>  >> >> >> On 23 June 2021 19:38:13 BST, Xinbin Huang 
>> <bin.huangxb@gmail.com <ma...@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 
>> <jarek@potiuk.com <ma...@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

Re: [VOTE] Disable pylint ?

[Jarek Potiuk <ja...@potiuk.com>]

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> 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 <tu...@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> 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 <ka...@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> wrote:
> >> >>
> >> >> Why not +10000 Ash :) ?
> >> >>
> >> >> On Wed, Jun 23, 2021 at 9:00 PM Andrew Godwin
> >> >> <an...@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 <as...@apache.org> wrote:
> >> >> >>
> >> >> >> Clearly I'm +7000 on this.
> >> >> >>
> >> >> >> On 23 June 2021 19:38:13 BST, Xinbin Huang <bi...@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> 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

Re: [VOTE] Disable pylint ?

[Jarek Potiuk <ja...@potiuk.com>]

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 <tu...@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> 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 <ka...@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> wrote:
>> >>
>> >> Why not +10000 Ash :) ?
>> >>
>> >> On Wed, Jun 23, 2021 at 9:00 PM Andrew Godwin
>> >> <an...@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 <as...@apache.org> wrote:
>> >> >>
>> >> >> Clearly I'm +7000 on this.
>> >> >>
>> >> >> On 23 June 2021 19:38:13 BST, Xinbin Huang <bi...@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> 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

Re: [VOTE] Disable pylint ?

[Tomasz Urbaszek <tu...@apache.org>]

-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
<https://en.wikipedia.org/wiki/Survivorship_bias#/media/File:Survivorship-bias.svg>
)?

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> 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 <ka...@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> wrote:
> >>
> >> Why not +10000 Ash :) ?
> >>
> >> On Wed, Jun 23, 2021 at 9:00 PM Andrew Godwin
> >> <an...@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 <as...@apache.org>
> wrote:
> >> >>
> >> >> Clearly I'm +7000 on this.
> >> >>
> >> >> On 23 June 2021 19:38:13 BST, Xinbin Huang <bi...@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>
> 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
>

Re: [VOTE] Disable pylint ?

[Jarek Potiuk <ja...@potiuk.com>]

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 <ka...@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> wrote:
>>
>> Why not +10000 Ash :) ?
>>
>> On Wed, Jun 23, 2021 at 9:00 PM Andrew Godwin
>> <an...@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 <as...@apache.org> wrote:
>> >>
>> >> Clearly I'm +7000 on this.
>> >>
>> >> On 23 June 2021 19:38:13 BST, Xinbin Huang <bi...@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> 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

Re: [VOTE] Disable pylint ?

[Kaxil Naik <ka...@gmail.com>]

+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> wrote:

> Why not +10000 Ash :) ?
>
> On Wed, Jun 23, 2021 at 9:00 PM Andrew Godwin
> <an...@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 <as...@apache.org>
> wrote:
> >>
> >> Clearly I'm +7000 on this.
> >>
> >> On 23 June 2021 19:38:13 BST, Xinbin Huang <bi...@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>
> 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
>

Re: [VOTE] Disable pylint ?

[Jarek Potiuk <ja...@potiuk.com>]

Why not +10000 Ash :) ?

On Wed, Jun 23, 2021 at 9:00 PM Andrew Godwin
<an...@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 <as...@apache.org> wrote:
>>
>> Clearly I'm +7000 on this.
>>
>> On 23 June 2021 19:38:13 BST, Xinbin Huang <bi...@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> 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

Re: [VOTE] Disable pylint ?

[Andrew Godwin <an...@astronomer.io.INVALID>]

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 <as...@apache.org> wrote:

> Clearly I'm +7000 on this.
>
> On 23 June 2021 19:38:13 BST, Xinbin Huang <bi...@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> 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
>>>
>>

Re: [VOTE] Disable pylint ?

[Ash Berlin-Taylor <as...@apache.org>]

Clearly I'm +7000 on this.

On 23 June 2021 19:38:13 BST, Xinbin Huang <bi...@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> 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
>>

Re: [VOTE] Disable pylint ?

[Jarek Potiuk <ja...@potiuk.com>]

Cyclic imports are detected by python (they do depend a bit on a
sequence of importing stuff).

We do have cyclic dependencies instead that pylint detected, but
solving it seems to be a high price to pay for all the worries of new
contributors who get some random errors.

I was one of the biggest proponents of solving those cyclic
dependencies, but looking at some recent problems of various people, I
honestly think it's not worth it at the end :).
Call it growing-up or maturing. :D. I feel I have grown up a bit by
seeing the consequences of it.

J.

On Wed, Jun 23, 2021 at 8:38 PM Xinbin Huang <bi...@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> 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

Re: [VOTE] Disable pylint ?

[Xinbin Huang <bi...@gmail.com>]

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> 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
>