Update `astroid` to 2.12 #7153

jacobtylerwalls · 2022-07-09T16:21:59Z

Type of Changes

	Type
✓	🔨 Refactoring

Fixes #5935
(among other issues to be closed shortly with regression tests in separate PRs, see label "Needs astroid update")

The main change relevant now is astroid's better support for detecting namespace packages.

coveralls · 2022-07-09T18:03:19Z

Pull Request Test Coverage Report for Build 2663062215

9 of 9 (100.0%) changed or added relevant lines in 2 files are covered.
5 unchanged lines in 4 files lost coverage.
Overall coverage decreased (-0.02%) to 95.371%

Files with Coverage Reduction	New Missed Lines	%
pylint/checkers/classes/class_checker.py	1	94.82%
pylint/lint/pylinter.py	1	94.74%
pylint/reporters/text.py	1	85.07%
pylint/lint/expand_modules.py	2	94.74%

Totals
Change from base Build 2662295713:	-0.02%
Covered Lines:	16792
Relevant Lines:	17607

💛 - Coveralls

jacobtylerwalls · 2022-07-09T22:05:34Z

@Pierre-Sassoulas It's more clear to me now: I think it's a packaging problem with https://github.com/Pierre-Sassoulas/contributors-txt, which installs a tests directory in site-packages.

>>> import tests
>>> tests
<module 'tests' from '/Users/.../new/lib/python3.10/site-packages/tests/__init__.py'>

The reason we don't have failures on Linux/MacOS is that those install requirements_test.txt with contributors-txt, and the pypy/windows workflows install requirements_test_min.txt without it. I'll push changes to bless the windows/pypy results and then the failures will occur on Linux/MacOS until fixing the packaging issue.

Would you be willing to look into the packaging issue?

Pierre-Sassoulas · 2022-07-10T05:52:29Z

Would you be willing to look into the packaging issue?

Sure, I'm on it!

doc/whatsnew/2/2.15/index.rst

pyproject.toml

requirements_test_min.txt

Pierre-Sassoulas · 2022-07-10T13:00:57Z

I released a new version of contributors-txt without tests directory. But I think that it's probably better if we patch the sys.path in the current test instead of changing the expected result. (Might be a good thing if we do it in another merge request prior to upgrading astroid and also remove our custom patching function in favor of the current pytest way to do it)

jacobtylerwalls · 2022-07-10T13:03:07Z

I wondered about that. Would you be willing to start that PR?

Pierre-Sassoulas · 2022-07-10T13:08:31Z

Yeap, let me try something :)

jacobtylerwalls · 2022-07-10T18:34:13Z

TODO:

decide if the adjusted test results in 9f67550 and d8d93d6 are tolerable (since tests is a package after all)
fix recursion failure with keras, see Catch RecursionError for delayed assattr nodes astroid#1660 (comment)

AdamWill · 2022-07-11T18:44:07Z

An interesting thing about the cases where we now get a tests. prefix on some strings: this changes depending on where you run the tests from. If you run them from outside the top level of the source tree - e.g. go up one level and run something like pytest pylint/tests - the tests. prefix goes away again (and the tests fail if they've been patched to expect it).

Not sure how much you care about this, but it's there. :D I found it while tinkering about to fix a different problem with running the tests as part of a package build (astroid doesn't like it if you run the tests from the top level of the source tarball but with PYTHONPATH=/path/to/installroot to try and run the tests using the 'installed' files, rather than the files from the tarball).

jacobtylerwalls · 2022-07-12T00:26:18Z

An interesting thing about the cases where we now get a tests. prefix on some strings: this changes depending on where you run the tests from.

Very true. And because that's true, we could probably avoid fiddling with the expected results and just change cwd to tests/data instead (as some other tests do). Thanks for the inspiration!

@DudeNr33 as a pyreverse expert does that sound like a good plan? Elsewhere you suggested (for new tests) what I think is a good practice of using dynamically generated files.

tests/pyreverse/test_diadefs.py

AdamWill · 2022-07-12T15:20:27Z

tests/messages/func_w0401_package.txt

@@ -1,2 +1,2 @@
-R:  1: Cyclic import (input.func_w0401_package.all_the_things -> input.func_w0401_package.thing2)
+R:  1: Cyclic import (tests.input.func_w0401_package.all_the_things -> tests.input.func_w0401_package.thing2)


This is the same thing as the pyreverse stuff, I think - should we change to the cwd-based fix here too?

cwd-based fix in 6fa6e74

DudeNr33 · 2022-07-12T17:02:46Z

An interesting thing about the cases where we now get a tests. prefix on some strings: this changes depending on where you run the tests from.

Very true. And because that's true, we could probably avoid fiddling with the expected results and just change cwd to tests/data instead (as some other tests do). Thanks for the inspiration!

@DudeNr33 as a pyreverse expert does that sound like a good plan? Elsewhere you suggested (for new tests) what I think is a good practice of using dynamically generated files.

Yes, that's fine for me! In reality I'd also expect the user to first cd into the root directory for his project.

DudeNr33

I only have an open question why two of the pyreverse tests are now marked as xfail - but as we have a lot of other tests for pyreverse I think this should not be a blocker, I'm just curious. 😊

DudeNr33 · 2022-07-12T17:08:50Z

doc/user_guide/usage/run.rst

+As of version 2.15, ``pylint`` will also discover namespace packages but there are some edge
+cases still under development (where you might still find ``--recursive=y`` useful).


I don't quite understand this sentence. From the wording it sounds like it is (in most cases) no longer necessary to use --recursive=y, yet the preceding sentences all describe the usage with it.

Improved comprehensibility in 63d654c

DudeNr33 · 2022-07-12T17:13:45Z

tests/pyreverse/test_diadefs.py

-        self, HANDLER: DiadefsHandler, PROJECT: Project
-    ) -> None:
+    # https://github.com/PyCQA/pylint/issues/2763
+    @pytest.mark.xfail


I haven't tried this myself yet, but why is the xfail necessary here?
The linked issue is about working without __init__.py files - but the tests/data has an __init__.py file?

It's possible I didn't understand the issue. (A good sign I should remove the comment!) I thought since tests doesn't have an __init__.py it's possible it would explain why the analysis of tests/data was not entirely correct. But maybe it's a manifestation of something else. I can remove the comment.

The failure is that only the last member of _should_rels is found, not all 5.

Removed comments in 916e556

Prevent `unused-import` for `six.with_metaclass` Update `contributors-txt` to 0.9.0 Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com> Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>

jacobtylerwalls · 2022-07-13T11:27:15Z

Thanks to all for helping push this over the line!

str's repr is the string, and you don't need a string call in a fstring. Follow-up to pylint-dev#7153 (comment)

github-actions · 2022-07-13T12:01:01Z

🤖 Effect of this PR on checked open source code: 🤖

Effect on django:
The following messages are now emitted:

invalid-name:
Attribute name "Meta" doesn't conform to '[a-z_][a-z0-9_]{2,}$' pattern
https://github.com/django/django/blob/main/django/db/models/base.py#L360
no-value-for-parameter:
No value for argument 'cls' in classmethod call
https://github.com/django/django/blob/main/django/utils/deconstruct.py#L17

The following messages are no longer emitted:

no-member:
Instance of 'BaseManager' has no '_constructor_args' member
https://github.com/django/django/blob/main/django/db/models/manager.py#L75
no-member:
Instance of 'BaseManager' has no '_constructor_args' member
https://github.com/django/django/blob/main/django/db/models/manager.py#L76
no-member:
Instance of 'BaseManager' has no '_constructor_args' member
https://github.com/django/django/blob/main/django/db/models/manager.py#L169

Effect on pandas:
The following messages are now emitted:

redefined-variable-type:
Redefinition of columns type from pandas.core.indexes.range.RangeIndex to pandas.core.indexes.base.Index
https://github.com/pandas-dev/pandas/blob/main/pandas/core/internals/construction.py#L860
redefined-variable-type:
Redefinition of new_target type from pandas.core.indexes.category.CategoricalIndex to pandas.core.indexes.base.Index
https://github.com/pandas-dev/pandas/blob/main/pandas/core/indexes/category.py#L455

The following messages are no longer emitted:

redefined-variable-type:
Redefinition of index type from pandas.core.indexes.base.Index to pandas.core.indexes.range.RangeIndex
https://github.com/pandas-dev/pandas/blob/main/pandas/core/series.py#L456
redefined-variable-type:
Redefinition of columns type from pandas.core.indexes.base.Index to pandas.core.indexes.range.RangeIndex
https://github.com/pandas-dev/pandas/blob/main/pandas/core/internals/construction.py#L850
empty-docstring:
Empty class docstring
https://github.com/pandas-dev/pandas/blob/main/pandas/core/arrays/floating.py#L141
empty-docstring:
Empty class docstring
https://github.com/pandas-dev/pandas/blob/main/pandas/core/arrays/floating.py#L148
empty-docstring:
Empty class docstring
https://github.com/pandas-dev/pandas/blob/main/pandas/core/arrays/integer.py#L160
empty-docstring:
Empty class docstring
https://github.com/pandas-dev/pandas/blob/main/pandas/core/arrays/integer.py#L167
empty-docstring:
Empty class docstring
https://github.com/pandas-dev/pandas/blob/main/pandas/core/arrays/integer.py#L174
empty-docstring:
Empty class docstring
https://github.com/pandas-dev/pandas/blob/main/pandas/core/arrays/integer.py#L181
empty-docstring:
Empty class docstring
https://github.com/pandas-dev/pandas/blob/main/pandas/core/arrays/integer.py#L188
empty-docstring:
Empty class docstring
https://github.com/pandas-dev/pandas/blob/main/pandas/core/arrays/integer.py#L195
empty-docstring:
Empty class docstring
https://github.com/pandas-dev/pandas/blob/main/pandas/core/arrays/integer.py#L202
empty-docstring:
Empty class docstring
https://github.com/pandas-dev/pandas/blob/main/pandas/core/arrays/integer.py#L209
redefined-variable-type:
Redefinition of new_target type from pandas.core.indexes.base.Index to pandas.core.indexes.category.CategoricalIndex
https://github.com/pandas-dev/pandas/blob/main/pandas/core/indexes/category.py#L442

Effect on pytest:
The following messages are now emitted:

consider-using-dict-items:
Consider iterating with .items()
https://github.com/pytest-dev/pytest/blob/main/src/_pytest/reports.py#L513
redefined-variable-type:
Redefinition of ret type from str to _pytest.config.ExitCode
https://github.com/pytest-dev/pytest/blob/main/src/_pytest/pytester.py#L1145
redefined-variable-type:
Redefinition of ret type from int to _pytest.config.ExitCode
https://github.com/pytest-dev/pytest/blob/main/src/_pytest/pytester.py#L1429
no-name-in-module:
No name 'PytestReturnNotNoneWarning' in module '_pytest.warning_types'
https://github.com/pytest-dev/pytest/blob/main/src/_pytest/python.py#L80
redefined-variable-type:
Redefinition of importhook type from _distutils_hack.DistutilsMetaFinder to _pytest.assertion.DummyRewriteHook
https://github.com/pytest-dev/pytest/blob/main/src/_pytest/assertion/__init__.py#L69

Effect on sentry:
The following messages are now emitted:

redefined-variable-type:
Redefinition of culprit type from str to list
https://github.com/getsentry/sentry/blob/master/src/sentry/culprit.py#L38
consider-iterating-dictionary:
Consider iterating the dictionary directly instead of calling .keys()
https://github.com/getsentry/sentry/blob/master/src/sentry/web/frontend/error_page_embed.py#L138
no-member:
Instance of 'module' has no 'file' member
https://github.com/getsentry/sentry/blob/master/src/sentry/utils/distutils/commands/base.py#L37

The following messages are no longer emitted:

unused-import:
Unused ActorTuple imported from sentry.models
https://github.com/getsentry/sentry/blob/master/src/sentry/models/projectownership.py#L9
not-callable:
fmt is not callable
https://github.com/getsentry/sentry/blob/master/src/sentry/utils/snuba.py#L1316
no-member:
Instance of 'str' has no 'objects' member
https://github.com/getsentry/sentry/blob/master/src/sentry/utils/snuba.py#L1316
redefined-variable-type:
Redefinition of initials type from str to django.utils.safestring.SafeString
https://github.com/getsentry/sentry/blob/master/src/sentry/utils/avatar.py#L77

This comment was generated for commit 2b4123a

…light from pylint-dev#7153

jacobtylerwalls added Astroid Related to astroid Maintenance Discussion or action around maintaining pylint or the dev workflow labels Jul 9, 2022

jacobtylerwalls added this to the 2.15.0 milestone Jul 9, 2022

jacobtylerwalls linked an issue Jul 9, 2022 that may be closed by this pull request

Update tests for bleeding-edge astroid namespace package changes #7058

Closed