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

Add typing to references of stream and stream() #4983

Merged
merged 2 commits into from
Sep 13, 2021

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

The next instalment of breaking up #4954 into more manageable PR's.
This PR is based on two related commits which I felt should be part of the same PR as they deal with a similar topic.

The first is annotation of all implementations of process_module. This is fairly straightforward and covers adding nodes.Module to the typing of the parameter and changing module, astroid and other parameter names to the new standard of node. As process_module calls node.stream() often this seemed like a good first step to this PR.

The second commit covers annotation of (most) functions with a reference to stream, filestream or a similar parameter. There are two important comments about this commit.

The first is for pylint/checkers/similar.py where I choose to use STREAM_TYPES = Union[TextIO, BufferedReader, BytesIO]. I did this because there seems to be a certain particularity with TextIO and IOBase. See the (small) discussion here: python/typeshed#3225 In practice this means that return types of open("b") and open("t") are incompatible. This became problematic and is why I opted to introduce STREAM_TYPES. This also means that typing is a little more restrictive than using IOBase although in practice this probably won't make a real difference.

The second is that in pylint/utils/utils.py I don't fully type the options parameter and use Tuple. This is because trying to determine the type of the second argument of this tuple became difficult and I didn't want to overstretch this PR. If we turn on some of the more stricter flags of mypy in the future this will probably show up again and should be solved then.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1214721205

  • 55 of 60 (91.67%) changed or added relevant lines in 13 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 93.13%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pylint/checkers/misc.py 8 9 88.89%
pylint/checkers/similar.py 7 8 87.5%
pylint/config/man_help_formatter.py 0 1 0.0%
pylint/lint/pylinter.py 7 8 87.5%
pylint/testutils/lint_module_test.py 7 8 87.5%
Totals Coverage Status
Change from base Build 1214643287: 0.02%
Covered Lines: 13258
Relevant Lines: 14236

πŸ’› - Coveralls

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

Lgtm :)

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

πŸ‘

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.11.0 milestone Sep 13, 2021
@Pierre-Sassoulas Pierre-Sassoulas added the Maintenance Discussion or action around maintaining pylint or the dev workflow label Sep 13, 2021
@Pierre-Sassoulas Pierre-Sassoulas merged commit c17457f into pylint-dev:main Sep 13, 2021
@DanielNoord DanielNoord deleted the typing-stream branch September 13, 2021 11:37
@DanielNoord DanielNoord mentioned this pull request Sep 24, 2021
2 tasks
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants