Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start requiring Python 3.6.2 or newer #5065

Closed
cdce8p opened this issue Sep 22, 2021 · 24 comments · Fixed by #5068
Closed

Start requiring Python 3.6.2 or newer #5065

cdce8p opened this issue Sep 22, 2021 · 24 comments · Fixed by #5068
Labels
Discussion 🤔 Maintenance Discussion or action around maintaining pylint or the dev workflow
Milestone

Comments

@cdce8p
Copy link
Member

cdce8p commented Sep 22, 2021

I would like to propose to drop support for Python 3.6.0 and 3.6.1 a bit ahead of schedule. This would allow us to use some typing feature added in 3.6.1 and 3.6.2 while still maintaining general support for Python 3.6 until its end of support in December.

Python 3.6.1

  • Default values for NamedTuples
  • ChainMap, Counter, Deque, AsyncGenerator

Python 3.6.2

  • NoReturn
  • AsyncContextManager

--
At the moment, we already use Counter and NoReturn wrapped in if TypeChecking or if sys.version_info checks.
There are good usecases for default values in NamedTuples as well. Especially with the ongoing refactoring effort.

--
For reference: #4310

@cdce8p cdce8p added Discussion 🤔 Maintenance Discussion or action around maintaining pylint or the dev workflow labels Sep 22, 2021
@cdce8p
Copy link
Member Author

cdce8p commented Sep 22, 2021

/CC: @Pierre-Sassoulas, @hippo91, @DanielNoord

@DanielNoord
Copy link
Collaborator

Is there any reliable way to get user statistics? I saw some discussion in the referenced issue and this would be helpful.

I am relatively new to the python (and pylint) community so my main question would be if there are any real obstacles which make it impossible for user to upgrade to 3.6.2 from 3.6.0. Considering the semversion, I don't suppose so but people with more experience might have more educated answers to this. If there are no real obstacles I'd we can go ahead.

@DudeNr33
Copy link
Collaborator

There are some situations where upgrading a Python version, even just a small step like this, is difficult.
For example if you work on an embedded device, or if you use it in commercial applications and would need to repeat an OSS clearing if you update the version.

Both points deal more with production code, but there can be instances where users want to use the exact same Python version in their test environment (this makes at least very good sense for dynamic tests).

But the real question is, if those corner cases should hinder us from dropping the support.
A) I don't think it will affect that much users
B) 3.6 reaches its EOL in just 3 months, so this does not really make a difference

@Pierre-Sassoulas
Copy link
Member

I'm on mobile so I'll try to stay short.

I think black did it, so we can probably too. Also the pipystats link in the old issue is still valid and show a dramatic drop in 3.6 usage recentely (New Debian with 3.9 ?) which probably indicate that < 3.6.2 usage is even lower now.

@cdce8p
Copy link
Member Author

cdce8p commented Sep 23, 2021

The black PR with which they started requiring 3.6.2: psf/black#1668
From what I can tell their reasons have been similar to the once I mentioned in the beginning: Especially the support for NoReturn and default values for NamedTuples.

If we reach a conclusion, I would suggest to drop support in 2.12.

@cdce8p
Copy link
Member Author

cdce8p commented Sep 23, 2021

@pkolbus You originally fixed the issues with 3.6.0 and Counter + NoReturn in #4446. Would this change work for you?

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.12.0 milestone Sep 23, 2021
@pkolbus
Copy link
Contributor

pkolbus commented Sep 23, 2021

@cdce8p, thanks for looping me in. Yes, with the update to python_requires, #5068 is workable; anyone using the old patch versions will just get the older pylint releases. (And our migration away from 3.6.0 is nearly complete, so we’ll hopefully be able to upgrade pylint and other packages soon).

@cdce8p
Copy link
Member Author

cdce8p commented Sep 23, 2021

@pkolbus Thanks for the feedback! Just out of curiosity: Are you still able to run pylint at the moment (on Python 3.6)? We discovered in #5070 that there might be some existing issues, in particular with NamedTuples.

@pkolbus
Copy link
Contributor

pkolbus commented Sep 27, 2021

@cdce8p I’m still pinning pylint to 2.7.x at the moment. Have deprioritized upgrading any packages for a few months now because these typing compatibility issues were just getting too widespread. (Pallets projects, and even pip).

@cdce8p
Copy link
Member Author

cdce8p commented Sep 29, 2021

@Pierre-Sassoulas and I had a small discussion about the next release. We have quite a few things planned which is way it doesn't necessarily feel right to release it as 2.11.2. Instead going for 2.12.0 directly might be the way to go.

Do we want to merge #5068 and #5070 then, or move them to 2.13?

--
#5094 (comment)

@DanielNoord
Copy link
Collaborator

DanielNoord commented Sep 29, 2021

Perhaps releasing one final version with all the bug fixes we have been working on for those stuck on 3.6.0 and 3.6.1 is best. To me it seems weird not to offer them those bug fixes while we still easily can.

Btw, I think it would be good to block the 2.11.2 milestone (or whatever version it will eventually be). New fixes can then target 2.11.3 (or 2.12.1). Right now 2.11.2 seems to be mostly waiting on astroid 2.8.1, so we should probably try and release that first.

@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Sep 29, 2021

I agree with the assessment about waiting for astroid. In fact almost all the time we release the hotfix concerning pylint alone very fast then we're stuck a long time with all the "require-astroid-update" issues that I try to group. Maybe we should release astroid more often so we can release pylint hot fixes in a timely manner without waiting for all the fixes.

@cdce8p
Copy link
Member Author

cdce8p commented Oct 4, 2021

Not sure about releasing astroid more often, but I like your suggestion about pylint 2.12 and 2.13. Should we rename the milestones then?

Fyi: I think we can still do some more work before releasing 2.12. That doesn't need to happen right away.

@Pierre-Sassoulas
Copy link
Member

I think an actual issue was reported for CPython 3.6.0 on the mailing list:

$ python3.6 -m pylint keepercommander/commands/record_common.py
cmd output started 2021 Mon Oct 04 03:54:33 PM PDT
Traceback (most recent call last):
  File "/usr/local/cpython-3.6/lib/python3.6/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/local/cpython-3.6/lib/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/Users/dstromberg/.local/lib/python3.6/site-packages/pylint/__main__.py", line 9, in <module>
    pylint.run_pylint()
  File "/Users/dstromberg/.local/lib/python3.6/site-packages/pylint/__init__.py", line 24, in run_pylint
    PylintRun(sys.argv[1:])
  File "/Users/dstromberg/.local/lib/python3.6/site-packages/pylint/lint/run.py", line 268, in __init__
    linter.load_default_plugins()
  File "/Users/dstromberg/.local/lib/python3.6/site-packages/pylint/lint/pylinter.py", line 547, in load_default_plugins
    checkers.initialize(self)
  File "/Users/dstromberg/.local/lib/python3.6/site-packages/pylint/checkers/__init__.py", line 84, in initialize
    register_plugins(linter, __path__[0])
  File "/Users/dstromberg/.local/lib/python3.6/site-packages/pylint/utils/utils.py", line 195, in register_plugins
    module.register(linter)
  File "/Users/dstromberg/.local/lib/python3.6/site-packages/pylint/checkers/refactoring/__init__.py", line 50, in register
    linter.register_checker(RefactoringChecker(linter))
  File "/Users/dstromberg/.local/lib/python3.6/site-packages/pylint/checkers/refactoring/refactoring_checker.py", line 468, in __init__
    self._consider_using_with_stack = ConsiderUsingWithStack()
TypeError: __new__() missing 3 required positional arguments: 'module_scope', 'class_scope', and 'function_scope'

It doesn't appear to matter what file I try to lint; pylint gives that same traceback.

Further detail:

$ python3.6 -m pylint --version
cmd output started 2021 Mon Oct 04 03:55:06 PM PDT
pylint 2.11.1
astroid 2.8.0
Python 3.6.0 (default, Oct  4 2021, 15:45:33)
[GCC Apple LLVM 12.0.5 (clang-1205.0.22.11)]
$ python3.6 --version
cmd output started 2021 Mon Oct 04 04:06:05 PM PDT
Python 3.6.0

@cdce8p
Copy link
Member Author

cdce8p commented Oct 5, 2021

I think an actual issue was reported for CPython 3.6.0 on the mailing list:

The issue is caused by a NamedTuple with default arguments and custom methods. This one here:
https://github.com/PyCQA/pylint/blob/f193806ed4eeb5d311523de54ff9fb83d64c57e9/pylint/checkers/refactoring/refactoring_checker.py#L160-L184

I mentioned it already here #5070 (comment), but I think the current state is broken for Python 3.6.0. That likely has been the case since pylint at least 2.9.4.

--
In the end, I'm not sure it would make much sense to release a new version without also requiring Python 3.6.2.

@Pierre-Sassoulas
Copy link
Member

Hmm, so the last version compatible with python 3.6 should be based on 2.9.3 ? Maybe even 2.8 ? How should we handle the releasing of the last compatible version for 3.6.0 ? We could release a 2.11.2 compatible with 3.6.0 and 3.6.1 based on 2.9.3 or even 2.8, after a 2.11.3 based on 2.11 but not compatible with python < 3.6.2. This seems hacky. We can of course make 2.11 compatible but this seems a lot of work (or we remove temporarily the offending typing?). Thought ?

@cdce8p
Copy link
Member Author

cdce8p commented Oct 5, 2021

Not sure we need to address this at all. Wouldn't it be enough to say Use Python 3.6.2+ or pylint 2.9.3 if it ever comes up? I suspect we don't have that many users who don't plan to upgrade soon anyway.

@Pierre-Sassoulas
Copy link
Member

The problem is that the doc is not read and the pip requirements are enforced. Maybe we can release a 2.11.2 compatible with 3.6.0 (according to pip) but then raise an error when actually using it with this python interpreter telling to install 2.9.3.

@cdce8p
Copy link
Member Author

cdce8p commented Oct 5, 2021

The problem is that the doc is not read and the pip requirements are enforced. Maybe we can release a 2.11.2 compatible with 3.6.0 (according to pip) but then raise an error when actually using it with this python interpreter telling to install 2.9.3.

That might be possible, but how many users will realistically be impacted by it? Pylint has been broken for a few months now (for 3.6.0) and so far we haven't gotten any bug report about it.

@Pierre-Sassoulas
Copy link
Member

Well the first report came in, so I guess we have at least one user affected and willing to take the time to signal the problem now. (https://mail.python.org/archives/list/code-quality@python.org/thread/3BLJIH4QKVONL3XTGIBADSEC4FQEGDUX/)

@cdce8p
Copy link
Member Author

cdce8p commented Oct 5, 2021

Well the first report came in, so I guess we have at least one user affected and willing to take the time to signal the problem now. (https://mail.python.org/archives/list/code-quality@python.org/thread/3BLJIH4QKVONL3XTGIBADSEC4FQEGDUX/)

We can't make everyone happy unfortunately. There will always be tradeoffs. The only clean solution would be to make fix the issue, but as you've already pointed out, that would be a lot of work.

I guess, I've already made up my mind here 🤷🏻‍♂️ Raising an error would be possible, but is there really a benefit to it for the additional work required?

@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Oct 6, 2021

I think if it's documented directly un pylint code it's better. If there is a crash without explanation chances are users will think pylint is broken and not bother reading the doc. The other solution to create a false 2.11 based on 2.9.3 so pip handle the version to recover automatically seems hacky.

Current updated TODO:
#5250
#5249

@cdce8p
Copy link
Member Author

cdce8p commented Oct 6, 2021

Just a followup: If we plan Python > 3.6.2 for 2.12.1, wouldn't it make sense to merge #5068 and #5070 for that? I think that would make sense, and we don't even need to release 2.13 as 2.12.1 handles it already.

After that we can just return to our "normal" release cycle.

@cdce8p cdce8p modified the milestones: 2.13.0, 2.12.1 Oct 6, 2021
Pierre-Sassoulas added a commit that referenced this issue Oct 17, 2021
Pierre-Sassoulas added a commit that referenced this issue Oct 17, 2021
Pierre-Sassoulas added a commit that referenced this issue Oct 17, 2021
* Add an exception for python < 3.6.2

See #5065 for reasoning

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

cdce8p commented Oct 17, 2021

Fyi: Added 2 more items to the todo list:

  • new section in whatsnew
  • Remove exception for 2.12.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion 🤔 Maintenance Discussion or action around maintaining pylint or the dev workflow
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants