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

Use python-typing-update on pylint/utils and message dirs #6304

Merged
merged 1 commit into from
Apr 14, 2022

Conversation

DanielNoord
Copy link
Collaborator

  • 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

Ref #4683.

@DanielNoord DanielNoord added this to the 2.14.0 milestone Apr 14, 2022
@DanielNoord DanielNoord added typing Maintenance Discussion or action around maintaining pylint or the dev workflow labels Apr 14, 2022
@DanielNoord DanielNoord requested a review from cdce8p April 14, 2022 08:16
Tuple,
Union,
)
from typing import TYPE_CHECKING, DefaultDict, Dict, Iterator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed Iterator and DefaultDict which could both be imported from collections.abc. For that to happen automatically, python-typing-update would need to be run with --py39-plus though. https://github.com/asottile/reorder_python_imports#rewriting-pep-585-typing-imports

Maybe that's worth doing after the initial round of changes, since it could also include incompatible changes. Otherwise pylint should also detect deprecated typing aliases once we update the py-version setting.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, yeah I was wondering about DefaultDict anyway. This PR includes a line where a defaultdict is typed as dict. That might also be something we want to change? Manual check of all defaultdicts wouldn't hurt I guess.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small update, for python-typing-update, both the --py39-plus and --full-reorder setting are necessary to rewrite all PEP 585 typing aliases. https://github.com/cdce8p/python-typing-update#configuration

However, this might also change runtime code (e.g. in type aliases). Might be best to just use the consider-using-alias check for it. (With py-version = 3.7.2)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I saw one too just now. Fine with me, let's do these changes first and see what pylint reports for us!

@DanielNoord DanielNoord merged commit 85a4804 into pylint-dev:main Apr 14, 2022
@DanielNoord DanielNoord deleted the 3.7 branch April 14, 2022 08:27
@coveralls
Copy link

Pull Request Test Coverage Report for Build 2166014898

  • 52 of 52 (100.0%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.003%) to 93.383%

Totals Coverage Status
Change from base Build 2166012630: 0.003%
Covered Lines: 15892
Relevant Lines: 17018

πŸ’› - Coveralls

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

Successfully merging this pull request may close these issues.

3 participants