PR#1565: Do a better job of ignoring pylint no-member errors - pagure

pagure

#1565 Do a better job of ignoring pylint no-member errors

Merged 7 years ago by pingou. Opened 7 years ago by adamwill.

adamwill/pagure no-member into master

Do a better job of ignoring pylint no-member errors

Adam Williamson • 7 years ago

4f81418

.pylintrc

file added

+22

		`@@ -0,0 +1,22 @@`
		`+ [TYPECHECK]`
		`+`
		`+ # add,bind,close,commit,delete,flush,rollback are for SQLAlchemy's`
		`+ # scoped_session implementation, which uses __call__ to pass the call`
		`+ # to a backing instance`
		`+`
		`+ # secure_filename is from werkzeug, which lazy loads functions from`
		`+ # submodules`
		`+`
		`+ # get_object,set_target,shorthand,target are for some kind of`
		`+ # shenanigans going on with flask and werkzeug proxying and pygit2`
		`+`
		`+ # GIT_REPOSITORY_INIT_SHARED_GROUP is for a constant we get from`
		`+ # pygit2; we could use:`
		`+ # pygit2.GIT_REPOSITORY_INIT_SHARED_GROUP`
		`+ # instead of:`
		`+ # pygit2.C.GIT_REPOSITORY_INIT_SHARED_GROUP`
		`+ # and avoid the pylint error, but per`
		`+ # https://github.com/libgit2/pygit2/issues/483`
		`+ # that only works since a commit in early 2015 which may be too new`
		`+ # to be safe`
		`+ generated-members=GIT_REPOSITORY_INIT_SHARED_GROUP,add,bind,close,commit,delete,flush,get_object,rollback,secure_filename,set_target,shorthand,target`

pagure/api/__init__.py

file modified

+1 -2

		`@@ -12,7 +12,6 @@`

		`# pylint: disable=invalid-name`
		`# pylint: disable=too-few-public-methods`
		`- # pylint: disable=no-member`
		`# pylint: disable=too-many-locals`

		`import codecs`
		`@@ -424,7 +423,7 @@`
		`}`

		`'''`
		`- errors = {val.name: val.value for val in APIERROR.__members__.values()}`
		`+ errors = {val.name: val.value for val in APIERROR.__members__.values()} # pylint: disable=no-member`
pingou commented 7 years ago Isn't there too many spaces before that `#`?

		`return flask.jsonify(errors)`

pagure/internal/__init__.py

file modified

-3

		`@@ -10,9 +10,6 @@`

		`"""`

		`- # pylint: disable=no-member`
		`-`
		`-`
		`import shutil`
		`import tempfile`
		`import os`

pagure/lib/git.py

file modified

-1

		`@@ -12,7 +12,6 @@`
		`# pylint: disable=too-many-arguments`
		`# pylint: disable=too-many-locals`
		`# pylint: disable=too-many-statements`
		`- # pylint: disable=no-member`
		`# pylint: disable=too-many-lines`

pagure/lib/model.py

file modified

-1

		`@@ -44,7 +44,6 @@`
		`# pylint: disable=invalid-name`
		`# pylint: disable=too-few-public-methods`
		`# pylint: disable=no-init`
		`- # pylint: disable=no-member`
		`# pylint: disable=too-many-lines`

pagure/ui/admin.py

file modified

-3

		`@@ -8,9 +8,6 @@`

		`"""`

		`- # pylint: disable=no-member`
		`-`
		`-`
		`from functools import wraps`

		`import flask`

pagure/ui/app.py

file modified

-4

		`@@ -23,10 +23,6 @@`
		`admin_session_timedout)`


		`- # Application`
		`- # pylint: disable=no-member`
		`-`
		`-`
		`@APP.route('/browse/projects', endpoint='browse_projects')`
		`@APP.route('/browse/projects/', endpoint='browse_projects')`
		`@APP.route('/')`

pagure/ui/fork.py

file modified

-1

		`@@ -14,7 +14,6 @@`
		`# pylint: disable=too-many-locals`
		`# pylint: disable=too-many-statements`
		`# pylint: disable=too-many-lines`
		`- # pylint: disable=no-member`


		`import flask`

pagure/ui/groups.py

file modified

-2

		`@@ -8,8 +8,6 @@`

		`"""`

		`- # pylint: disable=no-member`
		`-`
		`import flask`

		`from sqlalchemy.exc import SQLAlchemyError`

pagure/ui/issues.py

file modified

-1

		`@@ -8,7 +8,6 @@`

		`"""`

		`- # pylint: disable=no-member`
		`# pylint: disable=too-many-lines`
		`# pylint: disable=too-many-branches`
		`# pylint: disable=too-many-locals`

pagure/ui/login.py

file modified

-3

		`@@ -9,9 +9,6 @@`

		`"""`

		`- # pylint: disable=no-member`
		`-`
		`-`
		`import datetime`
		`import urlparse`

pagure/ui/plugins.py

file modified

-1

		`@@ -9,7 +9,6 @@`
		`"""`

		`# pylint: disable=too-many-branches`
		`- # pylint: disable=no-member`

		`import flask`

pagure/ui/repo.py

file modified

-1

		`@@ -8,7 +8,6 @@`

		`"""`

		`- # pylint: disable=no-member`
		`# pylint: disable=too-many-lines`
		`# pylint: disable=too-many-branches`
		`# pylint: disable=too-many-locals`

adamwill commented 7 years ago

Several files had pylint no-member disabled for the entire file
to avoid a few specific false positives for the test. This will
prevent us finding genuine bugs in those files. Instead, add a
project level .pylintrc which lists specific member names
that pylint has trouble with as generated-members.

The only risk with this is if there happens to be a genuine
no-member error for a different member with the same name as
one of these whitelisted members, but that seems like a smaller
risk than the risk of there being a genuine no-member error in
any of the files where we were turning the check off for the
entire file. This will also avoid the same errors in files which
didn't have a file-wide no-member, including all the tests.

Most of these false positives are caused by modules being clever
about stuff and may not be possible for pylint to fix, but one
does seem like a straight-up pylint bug, and I've reported it:

https://github.com/PyCQA/pylint/issues/1165

for that one I added a single-line disable of no-members instead
of including __members__ in the generated-members list.

adamwill commented 7 years ago

This assumes any run of pylint we care about will be done using the rc file, of course (i.e. out of pagure/ or with an appropriate --rcfile).

Running PYTHONPATH=pagure pylint pagure on master at present gives a score of 9.30 and 34 no-member messages (in files which didn't have a whole-file disable of the check). Running it with this change gives a score of 9.50 and no no-member messages.

pingou commented on line 14 of pagure/api/__init__.py 7 years ago

Isn't there too many spaces before that #?

adamwill commented 7 years ago

@pingou - apparently I can't reply to inline comments - PEP8 says: "Inline comments should be separated by at least two spaces from the statement." It doesn't say how many, just 'at least two'. So I just hit tab enough times to get more than one (my tab width is four spaces, as God intended).

If you want it to be two, I can change it, I guess.

adamwill commented 7 years ago

BTW, in case you're wondering whether we can specify generated-members at the code level, like we can disable and enable tests: no, we can't. Because we can't have nice things, I guess.

pingou commented 7 years ago

For some reasons, I thought it was exactly two, I stand corrected :)

Thanks!

Pull-Request has been merged by pingou

7 years ago

adamwill commented 7 years ago

Also, it doesn't seem like we can specify something like "add is OK, but only as a member of scoped_session instances". Which would be really helpful. Sigh.