Add typing to all calls to `self.stats` #4973

DanielNoord · 2021-09-06T10:42:05Z

Add yourself to CONTRIBUTORS if you are a new contributor.
Write a good description on what the PR does.

Type of Changes

	Type
✓	🔨 Refactoring

Description

All checkers inherit from a baseclass which has a stats attribute.
This attribute has a fairly unmanageable type, but the current typing includes all variations of the attribute.
Other changes not directly related to self.stats are due to mypy warnings.
This incorporate the feedback received in #4954

All checkers inherit from a baseclass which has a ``stats`` attribute. This attribute has a fairly unmanageable type, but the current typing includes all variations of the attribute. Other changes not directly related to ``self.stats`` are due to ``mypy``warnings. This incorporate the feedback received in pylint-dev#4954

coveralls · 2021-09-06T10:47:02Z

Pull Request Test Coverage Report for Build 1235382760

80 of 80 (100.0%) changed or added relevant lines in 16 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.01%) to 93.089%

Totals
Change from base Build 1235242213:	0.01%
Covered Lines:	13282
Relevant Lines:	14268

💛 - Coveralls

DanielNoord · 2021-09-06T10:54:20Z

pylint/reporters/multi_reporter.py

@@ -95,8 +95,14 @@ def on_set_current_module(self, module: str, filepath: Optional[AnyPath]) -> Non

    def on_close(
        self,
-        stats: Mapping[Any, Any],
-        previous_stats: Mapping[Any, Any],
+        stats: Dict[


I changed this to Dict instead of Mapping as its only ever called with self.stats and using Mapping gave a Mypy warning because of an incorrect argument type for one of the calls to on_close.

Pierre-Sassoulas

I was thinking about creating a class so the typing is simpler and we can unit test it easily.

DanielNoord · 2021-09-06T10:56:49Z

pylint/checkers/__init__.py

-def table_lines_from_stats(stats, old_stats, columns):
+def table_lines_from_stats(
+    stats: Dict[
+        str, Union[int, Counter[str], List, Dict[str, Union[int, str, Dict[str, int]]]]


The List included in the type of all stats comes from https://github.com/PyCQA/pylint/blob/40cc2ffd7887959157aaf469e09585ec2be7f528/pylint/checkers/imports.py#L469
This is then used in https://github.com/PyCQA/pylint/blob/40cc2ffd7887959157aaf469e09585ec2be7f528/pylint/graph.py#L170-L182

cdce8p

Not a full review just yet

pylint/checkers/__init__.py

DanielNoord · 2021-09-06T12:03:41Z

tests/pyreverse/test_writer.py

@@ -28,7 +28,7 @@
 from unittest.mock import Mock

 import pytest
-from conftest import PyreverseConfig  # type: ignore
+from conftest import PyreverseConfig  # type: ignore #pylint: disable=no-name-in-module


For some reason the import of conftest introduced in 2500086 failed the pre-commit.

Shouldn't this be from pylint.pyrevers.conftest import PyreverseConfig?

See discussion here: #4950 (comment)
Seems like we can't fix this without a (big) refactor

cdce8p

I still feel like I need to think about this a bit longer. Left some comments nevertheless.

pylint/checkers/__init__.py

pylint/checkers/base.py

pylint/checkers/raw_metrics.py

pylint/lint/parallel.py

pylint/lint/report_functions.py

pylint/typing.py

cdce8p · 2021-09-06T21:47:48Z

tests/pyreverse/test_writer.py

@@ -28,7 +28,7 @@
 from unittest.mock import Mock

 import pytest
-from conftest import PyreverseConfig  # type: ignore
+from conftest import PyreverseConfig  # type: ignore #pylint: disable=no-name-in-module


Shouldn't this be from pylint.pyrevers.conftest import PyreverseConfig?

DanielNoord · 2021-09-14T18:57:36Z

@Pierre-Sassoulas You might want to change the milestone on this (or other PRs). Same goes for #4980 in order for the current typing efforts of you and me not to become double-work.

Just to let you know, I didn't plan on opening any new typing PRs before the current ones are all merged. Probably best to work on getting the current ones merged first. Perhaps you could help me with a review of #4980 as it requires quite extensive knowledge of pylint 😄

pylint/lint/parallel.py

pylint/reporters/reports_handler_mix_in.py

pylint/reporters/ureports/nodes.py

pylint/typing.py

Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>

for more information, see https://pre-commit.ci

pylint/checkers/__init__.py

cdce8p

@Pierre-Sassoulas Would you mind taking a look at this? Overall I think it's good. Haven't checked each type tough.

Pierre-Sassoulas

I like the new class name, and this cleans up a lot of potential problem too 👍

Pierre-Sassoulas · 2021-09-15T06:38:50Z

Shouldn't this be from pylint.pyrevers.conftest import PyreverseConfig?

Yes, I opened #5010

DanielNoord commented Sep 6, 2021

View reviewed changes

Pierre-Sassoulas reviewed Sep 6, 2021

View reviewed changes

DanielNoord commented Sep 6, 2021

View reviewed changes

cdce8p requested changes Sep 6, 2021

View reviewed changes

pylint/checkers/__init__.py Outdated Show resolved Hide resolved

pylint/checkers/__init__.py Outdated Show resolved Hide resolved

DanielNoord added 3 commits September 6, 2021 13:52

Add CheckerStatistic class to pylint/typing

f782e2a

Guard typing.Counter import

0528ee5

Add disable for import of conftest

04706af

DanielNoord commented Sep 6, 2021

View reviewed changes

cdce8p reviewed Sep 6, 2021

View reviewed changes

Implement code review

0c45661

DanielNoord mentioned this pull request Sep 7, 2021

Add typing to filepath #4980

Merged

2 tasks

cdce8p added this to the 2.12.0 milestone Sep 8, 2021

DanielNoord mentioned this pull request Sep 14, 2021

Add typing in pylint.reporters #5004

Merged

cdce8p reviewed Sep 14, 2021

View reviewed changes

Apply suggestions from code review

d35f86a

Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>

Pierre-Sassoulas modified the milestones: 2.12.0, 2.11.0 Sep 14, 2021

cdce8p mentioned this pull request Sep 14, 2021

Change lines to lists #5009

Merged

Pierre-Sassoulas added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Sep 14, 2021

DanielNoord and others added 3 commits September 14, 2021 23:08

Merge branch 'main' into typing-stats

c365d8b

[pre-commit.ci] auto fixes from pre-commit.com hooks

ebddfa9

for more information, see https://pre-commit.ci

Add Any to kwargs

6d76ab7

cdce8p reviewed Sep 14, 2021

View reviewed changes

pylint/checkers/__init__.py Outdated Show resolved Hide resolved

cdce8p reviewed Sep 14, 2021

View reviewed changes

Add Iterable and return annotation

1841e65

Pierre-Sassoulas approved these changes Sep 15, 2021

View reviewed changes

Pierre-Sassoulas merged commit 22e56c0 into pylint-dev:main Sep 15, 2021

DanielNoord deleted the typing-stats branch September 15, 2021 18:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add typing to all calls to `self.stats` #4973

Add typing to all calls to `self.stats` #4973

DanielNoord commented Sep 6, 2021

coveralls commented Sep 6, 2021 •

edited

Loading

DanielNoord Sep 6, 2021

Pierre-Sassoulas left a comment

DanielNoord Sep 6, 2021

cdce8p left a comment

DanielNoord Sep 6, 2021

cdce8p Sep 6, 2021

DanielNoord Sep 6, 2021

cdce8p left a comment

cdce8p Sep 6, 2021

DanielNoord commented Sep 14, 2021

cdce8p left a comment

Pierre-Sassoulas left a comment

Pierre-Sassoulas commented Sep 15, 2021

Add typing to all calls to self.stats #4973

Add typing to all calls to self.stats #4973

Conversation

DanielNoord commented Sep 6, 2021

Type of Changes

Description

coveralls commented Sep 6, 2021 • edited Loading

Pull Request Test Coverage Report for Build 1235382760

💛 - Coveralls

DanielNoord Sep 6, 2021

Choose a reason for hiding this comment

Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

DanielNoord Sep 6, 2021

Choose a reason for hiding this comment

cdce8p left a comment

Choose a reason for hiding this comment

DanielNoord Sep 6, 2021

Choose a reason for hiding this comment

cdce8p Sep 6, 2021

Choose a reason for hiding this comment

DanielNoord Sep 6, 2021

Choose a reason for hiding this comment

cdce8p left a comment

Choose a reason for hiding this comment

cdce8p Sep 6, 2021

Choose a reason for hiding this comment

DanielNoord commented Sep 14, 2021

cdce8p left a comment

Choose a reason for hiding this comment

Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

Pierre-Sassoulas commented Sep 15, 2021

Add typing to all calls to `self.stats` #4973

Add typing to all calls to `self.stats` #4973

coveralls commented Sep 6, 2021 •

edited

Loading