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

Cleanup after we drop support for Python 3.6 #4683

Closed
6 tasks done
Tracked by #5513
Pierre-Sassoulas opened this issue Jul 7, 2021 · 12 comments · Fixed by #6421
Closed
6 tasks done
Tracked by #5513

Cleanup after we drop support for Python 3.6 #4683

Pierre-Sassoulas opened this issue Jul 7, 2021 · 12 comments · Fixed by #6421
Labels
Blocker 🙅 Blocks the next release Maintenance Discussion or action around maintaining pylint or the dev workflow
Milestone

Comments

@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Jul 7, 2021

When we do drop support because 3.6 has reached its end of life, we'll need to do some cleanup:

@cdce8p
Copy link
Member

cdce8p commented Jul 7, 2021

With the removal, we could also start using features that have been added in 3.7
The most relevant to us IMO would be:

  • Postponed evaluation of annotations (with from __future__ import annotations)
  • Dataclasses

https://www.python.org/downloads/release/python-370/
https://docs.python.org/3.7/whatsnew/3.7.html#new-features

@cdce8p cdce8p added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Jul 7, 2021
@cdce8p
Copy link
Member

cdce8p commented Jul 7, 2021

Just for reference: #4310 (Issue discussing when the removal of 3.6 should happen)

@Pierre-Sassoulas
Copy link
Member Author

Thank for the suggestion I added postponed evaluation to the list. Regarding dataclasses, could you explain the advantages compared to keeping NamedTuple for example ?

@cdce8p
Copy link
Member

cdce8p commented Jul 8, 2021

Dataclasses are essentially just classes. So compared to NamedTuple you can change attributes without creating a new instance of it. Where dataclasses really shine is for classes that just implement some boilerplate code (__init__ to assign all attributes to instance attrs, basic __str__, __repr__, __eg__, ...). Basically, they are meant for classes that mostly store data (hence the name). https://docs.python.org/3/library/dataclasses.html

Good examples where to use it can be found in the PR for the Similarity checker.
https://github.com/PyCQA/pylint/blob/65a20b6c5075e1f1756c1e6033c20b289b6edbbf/pylint/checkers/similar.py#L97-L114

https://github.com/PyCQA/pylint/blob/65a20b6c5075e1f1756c1e6033c20b289b6edbbf/pylint/checkers/similar.py#L163-L189

@Pierre-Sassoulas
Copy link
Member Author

I added dataclasses to the list :) I think it should be done in good intelligence though (ie: not everywhere for refactor's sake)

@cdce8p
Copy link
Member

cdce8p commented Jul 8, 2021

I added dataclasses to the list :) I think it should be done in good intelligence though (ie: not everywhere for refactor's sake)

Yeah, I completely agree. The initial list was more meant to show which new features we can take most advantage of with 3.7 as min requirement not necessarily that we should refactor old code because of it.

@DanielNoord
Copy link
Collaborator

@cdce8p Is your typing upgrade tool ready to use for pylint? Or will its default value of 3.8 be problematic for us (making it easier to do everything by hand)?

@cdce8p
Copy link
Member

cdce8p commented Apr 13, 2022

@cdce8p Is your typing upgrade tool ready to use for pylint? Or will its default value of 3.8 be problematic for us (making it easier to do everything by hand)?

I just released a new version 0.4.0 which also added an option for 3.7: --py37-plus. Form a quick test, it looks like it's good to go. It's probably best to add it as a temporary pre-commit hook. At least that's the easiest way I found.

  - repo: https://github.com/cdce8p/python-typing-update
    rev: v0.4.0
    hooks:
      - id: python-typing-update
        stages: [manual]
        additional_dependencies:
          - "black==22.3.0"
        args:
          - --py37-plus
          - --force
          - --keep-updates
          - --black
        files: ^(pylint)/.+\.py$  # might need to be changed
pre-commit run --hook-stage manual python-typing-update --all-files

@DanielNoord
Copy link
Collaborator

I'll take a look at this after argparse migration is finished!

@Pierre-Sassoulas
Copy link
Member Author

I'm for using the pre-commit hook, it's easy to reintroduce typing.List (could be just habits dying hard). Keeping the hook make sure the code is normalized, the same way we have pyupgrade all the time.

@DanielNoord
Copy link
Collaborator

I'll add it to the CI action that runs pre-commit pylint then. The action is quite slow (sorry Marc 😅) as it essentially runs a bunch of other formatters.

@cdce8p
Copy link
Member

cdce8p commented Apr 14, 2022

The action is quite slow (sorry Marc 😅) as it essentially runs a bunch of other formatters.

The tool wasn't designed to be fast 😄

I'm for using the pre-commit hook, it's easy to reintroduce typing.List (could be just habits dying hard). Keeping the hook make sure the code is normalized, the same way we have pyupgrade all the time.

The pylint typing extension is much better for that. -> consider-using-alias and consider-alternative-union-syntax checks. We just need to update py-version for that.

@DanielNoord DanielNoord added the Blocker 🙅 Blocks the next release label Apr 14, 2022
@DanielNoord DanielNoord changed the title Cleanup atfer we drop support for Python 3.6 Cleanup after we drop support for Python 3.6 Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker 🙅 Blocks the next release Maintenance Discussion or action around maintaining pylint or the dev workflow
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants