Drop support for Python 3.8 #9774

jacobtylerwalls · 2024-07-07T19:41:58Z

Remove support for launching pylint with Python 3.8.
Code that supports Python 3.8 can still be linted with the --py-version=3.8 setting.

Type of Changes

	Type
✓	🔨 Refactoring

codecov · 2024-07-07T19:56:47Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.81%. Comparing base (05a1950) to head (4b45192).
Report is 138 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9774      +/-   ##
==========================================
- Coverage   95.84%   95.81%   -0.04%     
==========================================
  Files         174      174              
  Lines       18878    18860      -18     
==========================================
- Hits        18094    18070      -24     
- Misses        784      790       +6

Files with missing lines	Coverage Δ
pylint/checkers/base/name_checker/checker.py	`98.63% <100.00%> (ø)`
pylint/checkers/imports.py	`94.86% <100.00%> (ø)`
pylint/checkers/method_args.py	`100.00% <ø> (ø)`
pylint/checkers/nested_min_max.py	`100.00% <100.00%> (ø)`
pylint/checkers/stdlib.py	`96.25% <100.00%> (-0.69%)`	⬇️
pylint/checkers/symilar.py	`96.30% <100.00%> (ø)`
pylint/checkers/utils.py	`96.06% <100.00%> (-0.10%)`	⬇️
pylint/checkers/variables.py	`97.16% <100.00%> (-0.15%)`	⬇️
pylint/config/argument.py	`100.00% <100.00%> (ø)`
pylint/config/config_file_parser.py	`100.00% <100.00%> (ø)`
... and 21 more

... and 1 file with indirect coverage changes

Pierre-Sassoulas

Looks really great. I'm not sure we should remove the functional tests for py38/py39 (unless it's actual max_pyver=3.8) ?

Pierre-Sassoulas · 2024-07-07T20:44:51Z

pylint/checkers/method_args.py

@@ -41,7 +41,6 @@ class MethodArgsChecker(BaseChecker):
            "positional-only-arguments-expected",
            "Emitted when positional-only arguments have been passed as keyword arguments. "
            "Remove the keywords for the affected arguments in the function call.",
-            {"minversion": (3, 8)},


Shouldn't we replace this with a py-version check ?

Hmm, but if my library supports 3.7 - 3.12, and I'm using a 3.8 function with positional-only arguments in the wrong manner, wouldn't I rather continue to see the message even if --py-version=3.7, because I certainly have bigger problems anyway if I'm using a 3.8+ function when I supposedly support 3.7?

I thought --py-version was most helpful for ensuring we silence suggestion-style messages that don't make sense on lower versions, like consider-using-f-string, but here we have a message that won't ever get raised on a lower version anyway, so I don't think we need to add a --py-version workaround for it.

I agree with Jacob's second comment. I don't think it will add much benefit to spend the time to add support for that edge case. Removing this line is fine.

pylint/message/message_definition_store.py

jacobtylerwalls · 2024-07-07T20:53:25Z

I'm not sure we should remove the functional tests for py38/py39 (unless it's actual max_pyver=3.8) ?

I may not be following exactly, but it's worth remembering this: max_pyver=3.9 means run on 3.8 and below. I'd rather that be what happens when you put max_pyver=3.8, but it is what it is 🤷 . So anything with max_pyver=3.9 won't run when 3.9 is the minimum version and can be deleted, is my understanding.

Pierre-Sassoulas · 2024-07-07T21:01:56Z

I may not be following exactly, but it's worth remembering this: max_pyver=3.9 means run on 3.8 and below. I'd rather that be what happens when you put max_pyver=3.8, but it is what it is 🤷 .

Yeah, didn't have that in mind either 😅 I'm feeling like making a breaking change, it's likely that (mainly) only us pylint maintainers use that atm. I never saw a plugin with functional tests in the wild. I'll re-review with that in mind later.

DanielNoord

Awesome job!

Just one comment from me.

There are some functional tests where it would have been nice to convert them to be messages when py-version==3.7 but that is a lot of work and I don't think is something we should focus our limited time on.

.github/workflows/tests.yaml

DanielNoord · 2024-07-08T07:09:22Z

pylint/checkers/method_args.py

@@ -41,7 +41,6 @@ class MethodArgsChecker(BaseChecker):
            "positional-only-arguments-expected",
            "Emitted when positional-only arguments have been passed as keyword arguments. "
            "Remove the keywords for the affected arguments in the function call.",
-            {"minversion": (3, 8)},


I agree with Jacob's second comment. I don't think it will add much benefit to spend the time to add support for that edge case. Removing this line is fine.

pylint/testutils/output_line.py

DanielNoord

🚀

jacobtylerwalls · 2024-07-08T14:12:11Z

Last push resolves merge conflict with #9777

jacobtylerwalls · 2024-07-08T18:10:49Z

Latest push to resolve merge conflict

Pierre-Sassoulas

Sorry I wasn't able to review faster.

I feel like we're loosing a lot of valuable interpreter specific knowledge that could be integrated in py-version when we remove functional tests that ran only for EOL interpreters without looking in detail at what's happening. We already did that in the past but I think it's makingpy-version kinda weak and 3.8 is easier to support than 3.7 because the ast's column/line is so much closer with other interpreters. I don't know if you looked at each test in detail and cleaned up the copy pastes or simple removed the one that ran only on EOL interpreters, Jacob ? There's a lot to review there I still didn't do it, but I feel like, for example, the carefully crafted tests about typing in python 3.8 from g/generic_alias/generic_alias_collections_py37.py brought a lot of value and the removal is going to hurt when someone open issues postfaced "I use py-version 3.8, pylint should not suggest impossible things for my interpreter, it was working fine in old version".

Pierre-Sassoulas · 2024-07-09T20:44:08Z

doc/development_guide/contributor_guide/tests/writing_test.rst

@@ -61,7 +61,6 @@ test runner. The following options are currently supported:

 - "min_pyver": Minimal python version required to run the test
 - "max_pyver": Python version from which the test won't be run. If the last supported version is 3.9 this setting should be set to 3.10.
- "min_pyver_end_position": Minimal python version required to check the end_line and end_column attributes of the message


Not sure we should remove this, sometime there's a new column/line in a new version. In particular when a bug is fixed or a nodes end is adjusted (in cpython). When this is the case we choose the newer version. This is still true in new interpreters. (Introduced when dealing with 3.9 only column change afair ?)

Thanks for the explanation, will revert.

Pierre-Sassoulas · 2024-07-09T20:49:16Z

pylint/testutils/lint_module_test.py

-        self._linter._arg_parser.add_argument(
-            "--min_pyver_end_position", type=parse_python_version, default=(3, 8)
-        )


Default was 3.8 and should become None

Leaving this as 3.8 for now because it's strongly typed as a tuple.

jacobtylerwalls · 2024-07-09T22:17:41Z

Thanks for the review Pierre, and sorry for rushing you.

I don't know if you looked at each test in detail and cleaned up the copy pastes or simple removed the one that ran only on EOL interpreters, Jacob ?

I just removed tests that ran only on EOL interpreters. If there are tests showing functionality that we can capture with py-version, I'm more than happy to make a new issue and itemize each one showing whether it's been investigated. It feels more "useful and optional" than dropping python 3.8, which I think we needed to do this summer, and I had a day to do it.

There are some functional tests where it would have been nice to convert them to be messages when py-version==3.7 but that is a lot of work and I don't think is something we should focus our limited time on.

This is where I was at too, but as I mentioned above I'm happy to stub out an issue to investigate. I have separate commits in this PR to make this easier. However...

for example, the carefully crafted tests about typing in python 3.8 from g/generic_alias/generic_alias_collections_py37.py brought a lot of value

I'm not sure I see what we can salvage here. How are we going to know to raise unsubscriptable-object when py-version is 3.8 or below? Won't that require astroid changes? We've never done that to my knowledge, but my imagination might be a little weak here :D

Python 3.8 was EOL on 2024-10-07 (see https://devguide.python.org/versions/). See https://github.com/nmlorg/metabot/actions/runs/11828441666/job/32959263533 (pylint-dev/pylint#9774).

jacobtylerwalls added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Jul 7, 2024

jacobtylerwalls added this to the 3.3.0 milestone Jul 7, 2024

jacobtylerwalls requested review from cdce8p, DudeNr33, DanielNoord and Pierre-Sassoulas as code owners July 7, 2024 19:41

jacobtylerwalls mentioned this pull request Jul 7, 2024

Cleanup test skips #9773

Merged

jacobtylerwalls mentioned this pull request Jul 7, 2024

Fix FN for invalid-name for type-annotated module constants #9771

Draft

Pierre-Sassoulas reviewed Jul 7, 2024

View reviewed changes

DanielNoord reviewed Jul 8, 2024

View reviewed changes

jacobtylerwalls force-pushed the drop-py38 branch from af1dc5a to 3654b84 Compare July 8, 2024 12:58

DanielNoord previously approved these changes Jul 8, 2024

View reviewed changes

jacobtylerwalls dismissed DanielNoord’s stale review via ce7698f July 8, 2024 14:11

jacobtylerwalls force-pushed the drop-py38 branch from 3654b84 to ce7698f Compare July 8, 2024 14:11

DanielNoord previously approved these changes Jul 8, 2024

View reviewed changes

jacobtylerwalls removed request for DudeNr33 and cdce8p July 8, 2024 14:25

jacobtylerwalls added 9 commits July 8, 2024 14:08

Remove PY38_PLUS and related constants

79a7b1e

Remove PY39_PLUS constant

d83fb24

Drop support for Python 3.8

83244c0

Simplify testutils

833762f

Remove version guards in test_messages_documentation.py

33cd7b7

Remove _is_relative_to() util

bd400b5

Remove tests that only ran on python 3.8

7a089b5

Remove min_pyver=3.9

4976d7c

ruff check pylint --fix --unsafe-fixes

2b1bbed

jacobtylerwalls added 5 commits July 8, 2024 14:10

Fix callable import

1d066c5

Additional cleanups

77d72d2

Update functional test output

b0f9ce0

Additional callable import fixes

0b33db5

Remove --min_pyver_end_position

4b45192

jacobtylerwalls dismissed DanielNoord’s stale review via 4b45192 July 8, 2024 18:10

jacobtylerwalls force-pushed the drop-py38 branch from ce7698f to 4b45192 Compare July 8, 2024 18:10

DanielNoord approved these changes Jul 9, 2024

View reviewed changes

jacobtylerwalls merged commit 52fe657 into main Jul 9, 2024
32 of 40 checks passed

jacobtylerwalls deleted the drop-py38 branch July 9, 2024 11:35

Pierre-Sassoulas reviewed Jul 9, 2024

View reviewed changes

jacobtylerwalls mentioned this pull request Jul 9, 2024

Revert "Remove --min_pyver_end_position" #9784

Merged

Pierre-Sassoulas mentioned this pull request Sep 20, 2024

Release branch 3.3.0 #9940

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop support for Python 3.8 #9774

Drop support for Python 3.8 #9774

jacobtylerwalls commented Jul 7, 2024

codecov bot commented Jul 7, 2024 •

edited

Loading

Pierre-Sassoulas left a comment

Pierre-Sassoulas Jul 7, 2024

jacobtylerwalls Jul 7, 2024

jacobtylerwalls Jul 7, 2024

DanielNoord Jul 8, 2024

jacobtylerwalls commented Jul 7, 2024

Pierre-Sassoulas commented Jul 7, 2024

DanielNoord left a comment

DanielNoord Jul 8, 2024

DanielNoord left a comment

jacobtylerwalls commented Jul 8, 2024

jacobtylerwalls commented Jul 8, 2024

Pierre-Sassoulas left a comment

Pierre-Sassoulas Jul 9, 2024 •

edited

Loading

jacobtylerwalls Jul 9, 2024

Pierre-Sassoulas Jul 9, 2024

jacobtylerwalls Jul 9, 2024

jacobtylerwalls commented Jul 9, 2024 •

edited

Loading

Drop support for Python 3.8 #9774

Drop support for Python 3.8 #9774

Conversation

jacobtylerwalls commented Jul 7, 2024

Type of Changes

codecov bot commented Jul 7, 2024 • edited Loading

Codecov Report

Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobtylerwalls commented Jul 7, 2024

Pierre-Sassoulas commented Jul 7, 2024

DanielNoord left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielNoord left a comment

Choose a reason for hiding this comment

jacobtylerwalls commented Jul 8, 2024

jacobtylerwalls commented Jul 8, 2024

Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

Pierre-Sassoulas Jul 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobtylerwalls commented Jul 9, 2024 • edited Loading

codecov bot commented Jul 7, 2024 •

edited

Loading

Pierre-Sassoulas Jul 9, 2024 •

edited

Loading

jacobtylerwalls commented Jul 9, 2024 •

edited

Loading