From 3bfd8660ffb06a7ceb5d42a7156d71abf5b5cb51 Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Tue, 19 Dec 2023 15:10:47 +0100 Subject: [PATCH] Put pylint and pylint django into place --- .github/workflows/ci.yml | 3 ++ .pre-commit-config.yaml | 21 +++++++++-- .pylintrc | 22 ------------ pyproject.toml | 36 +++++++++++++++++-- survey/__init__.py | 1 + survey/exporter/tex/survey2tex.py | 1 + survey/models/question.py | 9 ++--- survey/models/response.py | 8 ++--- survey/tests/base_test.py | 2 +- .../exporter/tex/test_question2tex_sankey.py | 23 ++++++------ survey/views/survey_detail.py | 2 +- 11 files changed, 77 insertions(+), 51 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7eaf0bf9..1422294d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,3 +32,6 @@ jobs: coverage run --source=survey --omit=survey/migrations/* ./manage.py test coverage html coveralls debug --service=github + - name: pylint + run: | + pre-commit run pylint --all-files diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 76ec1910..df4993de 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,5 +1,5 @@ -#ci: -# skip: [pylint] +ci: + skip: [pylint] repos: - repo: https://github.com/pre-commit/pre-commit-hooks @@ -29,3 +29,20 @@ repos: - id: prettier args: [--prose-wrap=always, --print-width=88] exclude: "survey/static/|dev/templates/|survey/templates/|survey/locale/|survey/tests/" + - repo: local + hooks: + - id: pylint + name: pylint + entry: pylint + language: system + types: [python] + args: + [ + "-rn", + "-sn", + "--rcfile=pyproject.toml", + "--django-settings-module=settings", + "--load-plugins=pylint_django", + "--fail-on=I", + ] + exclude: "survey/migrations/" diff --git a/.pylintrc b/.pylintrc index 7ac22821..e69de29b 100644 --- a/.pylintrc +++ b/.pylintrc @@ -1,22 +0,0 @@ -[MASTER] -# Python code to execute, usually for sys.path manipulation such as pygtk.require(). -init-hook=import sys; sys.path.insert(0, 'survey');sys.path.insert(0, 'venv/lib/python3.7/site-packages/'); - -[MESSAGES CONTROL] -# I0011 Warning locally suppressed using disable-msg -disable= - I0011, - no-member, - missing-docstring, # We don't want docstring everywhere - C0330, # black handle this - too-few-public-methods, # More harmful than beneficial in django project - too-many-arguments, - -ignore=migrations - -[BASIC] -# Good variable names which should always be accepted, separated by a comma. -good-names=i,j,k,ex,Run,_,f,e,maxDiff - -[FORMAT] -max-line-length=120 diff --git a/pyproject.toml b/pyproject.toml index dcda4cbe..1a48a791 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,6 +50,7 @@ dev = [ "coveralls", "colorama", "pylint", + "pylint-django", "pre-commit" ] sankey = [ @@ -59,8 +60,6 @@ sankey = [ [tool.setuptools.dynamic] dependencies = {file = "requirements.txt"} - - [tool.ruff] line-length = 120 @@ -78,3 +77,36 @@ select = [ ignore = [ "RUF012", # mutable default values in class attributes ] + +[tool.pylint.main] +load-plugins ="pylint_django" + +disable=[ + # I0011 Warning locally suppressed using disable-msg + "I0011", + "no-member", + "missing-docstring", # We don't want docstring everywhere + "too-few-public-methods", # More harmful than beneficial in django project + "too-many-arguments", + "too-many-instance-attributes", + # TODO Fix + "unspecified-encoding", + "inconsistent-return-statements", + "consider-using-with", + "fixme", + "no-else-return", + "imported-auth-user", + "unused-argument", + "arguments-differ", + "consider-using-f-string", + "too-many-branches", + "redefined-builtin", + "superfluous-parens", + "useless-parent-delegation", + "unused-private-member", + "duplicate-code", + "attribute-defined-outside-init", +] +ignore="migrations" +good-names="i,j,k,ex,Run,_,f,e,maxDiff" +max-line-length = 120 diff --git a/survey/__init__.py b/survey/__init__.py index 6175ddfa..463902ef 100644 --- a/survey/__init__.py +++ b/survey/__init__.py @@ -14,6 +14,7 @@ def set_default_settings(): try: + # pylint: disable=import-outside-toplevel from django.conf import settings from . import settings as app_settings diff --git a/survey/exporter/tex/survey2tex.py b/survey/exporter/tex/survey2tex.py index 90a07f76..c4d82883 100755 --- a/survey/exporter/tex/survey2tex.py +++ b/survey/exporter/tex/survey2tex.py @@ -48,6 +48,7 @@ def _additional_analysis(self, survey, latex_file): latex_file.text += function_(survey) def treat_question(self, question): + # pylint: disable=too-many-locals LOGGER.info("Treating, %s %s", question.pk, question.text) options = self.tconf.get(survey_name=self.survey.name, question_text=question.text) multiple_charts = options.get("multiple_charts") diff --git a/survey/models/question.py b/survey/models/question.py index 7b08720b..f557197d 100644 --- a/survey/models/question.py +++ b/survey/models/question.py @@ -268,6 +268,7 @@ def sorted_answers_cardinality( The ordering is reversed for same cardinality value so we have aa before zz.""" + # pylint: disable=too-many-locals cardinality = self.answers_cardinality( min_cardinality, group_together, group_by_letter_case, group_by_slugify, filter, other_question ) @@ -344,7 +345,7 @@ def __add_user_cardinality( filter, standardized_filter, ): - found_answer = False + values = [_(settings.USER_DID_NOT_ANSWER)] for other_answer in other_question.answers.all(): if user is None: break @@ -352,12 +353,8 @@ def __add_user_cardinality( # We suppose there is only a response per user # Why would you want this info if it is # possible to answer multiple time ? - found_answer = True + values = other_answer.values break - if found_answer: - values = other_answer.values - else: - values = [_(settings.USER_DID_NOT_ANSWER)] for other_value in values: other_value = self.__get_cardinality_value( other_value, group_by_letter_case, group_by_slugify, group_together diff --git a/survey/models/response.py b/survey/models/response.py index 41f63385..6d5c1425 100644 --- a/survey/models/response.py +++ b/survey/models/response.py @@ -8,11 +8,11 @@ from django.conf import settings if settings.AUTH_USER_MODEL: - user_model = settings.AUTH_USER_MODEL + UserModel = settings.AUTH_USER_MODEL else: - user_model = User + UserModel = User except (ImportError, AttributeError): - user_model = User + UserModel = User class Response(models.Model): @@ -25,7 +25,7 @@ class Response(models.Model): created = models.DateTimeField(_("Creation date"), auto_now_add=True) updated = models.DateTimeField(_("Update date"), auto_now=True) survey = models.ForeignKey(Survey, on_delete=models.CASCADE, verbose_name=_("Survey"), related_name="responses") - user = models.ForeignKey(user_model, on_delete=models.SET_NULL, verbose_name=_("User"), null=True, blank=True) + user = models.ForeignKey(UserModel, on_delete=models.SET_NULL, verbose_name=_("User"), null=True, blank=True) interview_uuid = models.CharField(_("Interview unique identifier"), max_length=36) class Meta: diff --git a/survey/tests/base_test.py b/survey/tests/base_test.py index 74e6ff09..2d0972f6 100755 --- a/survey/tests/base_test.py +++ b/survey/tests/base_test.py @@ -24,7 +24,7 @@ def login(self): """Log the user in.""" is_logged = self.client.login(username=settings.DEBUG_ADMIN_NAME, password=settings.DEBUG_ADMIN_PASSWORD) if not is_logged: # pragma: no cover - raise Exception("Login failed for test user! Tests won't work.") + raise ValueError("Login failed for test user! Tests won't work.") def logout(self): """Log the user out.""" diff --git a/survey/tests/exporter/tex/test_question2tex_sankey.py b/survey/tests/exporter/tex/test_question2tex_sankey.py index ecd71e77..cf1dbc61 100755 --- a/survey/tests/exporter/tex/test_question2tex_sankey.py +++ b/survey/tests/exporter/tex/test_question2tex_sankey.py @@ -11,16 +11,13 @@ def test_other_question_type(self): q2s = Question2TexSankey(question, other_question=other_question) self.assertIsNotNone(q2s.tex()) - -""" - def test_big_ranking_survey(self): - # Creating a big ranking survey with user takes a long time - self.create_big_ranking_survey(with_user=True) - qtext = "How much do you like question {} ?" - from survey.models import Question - - q4 = Question.objects.get(text=qtext.format(4)) - q5 = Question.objects.get(text=qtext.format(5)) - q2tex_sankey = Question2TexSankey(q4, filter=["1"], other_question=q5, group_together={"A": ["2", "3"]}) - q2tex_sankey.tex() -""" + # def test_big_ranking_survey(self): + # # Creating a big ranking survey with user takes a long time + # self.create_big_ranking_survey(with_user=True) + # qtext = "How much do you like question {} ?" + # from survey.models import Question + # + # q4 = Question.objects.get(text=qtext.format(4)) + # q5 = Question.objects.get(text=qtext.format(5)) + # q2tex_sankey = Question2TexSankey(q4, filter=["1"], other_question=q5, group_together={"A": ["2", "3"]}) + # q2tex_sankey.tex() diff --git a/survey/views/survey_detail.py b/survey/views/survey_detail.py index 61fa8157..68b6335e 100644 --- a/survey/views/survey_detail.py +++ b/survey/views/survey_detail.py @@ -30,7 +30,7 @@ def get(self, request, *args, **kwargs): asset_context = { # If any of the widgets of the current form has a "date" class, flatpickr will be loaded into the template - "flatpickr": any([field.widget.attrs.get("class") == "date" for _, field in form.fields.items()]) + "flatpickr": any(field.widget.attrs.get("class") == "date" for _, field in form.fields.items()) } context = { "response_form": form,