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 half of the tests directory #6317

Merged
merged 6 commits 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
@coveralls
Copy link

coveralls commented Apr 14, 2022

Pull Request Test Coverage Report for Build 2168507121

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.373%

Totals Coverage Status
Change from base Build 2168404955: 0.0%
Covered Lines: 15964
Relevant Lines: 17097

💛 - Coveralls

Comment on lines 17 to 18
INPUT_DIR = join(dirname(__file__), "input")
MSG_DIR = join(dirname(__file__), "messages")
Copy link
Member

Choose a reason for hiding this comment

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

Not that it's a problem, but there's unrelated change there ?

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 noticed it too. It's probably from an of the upgrade tools in Marc's tool updating this as well due to the 3.9+ flag.
However, I think it is actually a good change 😄

Copy link
Member

Choose a reason for hiding this comment

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

Hmm that's suspicious though, in python 3.8 __file__ is not abspathed:

cat a.py    
print(__file__)
python3 a.py
a.py
cat a.py 
import os

print(os.path.abspath(__file__))
python3 a.py
/home/pierre/astroid/a.py

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm is it related to dirname perhaps?

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas Apr 14, 2022

Choose a reason for hiding this comment

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

Ho, it's actually worst if we take dirname into account:

cat a.py    
import os

print(os.path.dirname(__file__))
python3 a.py
 #nothing printed
cat a.py    
import os

print(os.path.dirname(os.path.abspath(__file__)))
python3 a.py
/home/pierre/astroid

(This is python 3.8)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we want to revert?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's safer.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's safer.

I agree. Let's revert it here.

@DanielNoord DanielNoord marked this pull request as ready for review April 14, 2022 13:35
Copy link
Member

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

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

Let's just focus on the typing changes here.
As part of python-typing-update, pyupgrade is run. The version setting is passed directly to it. If those updates depend on --py39-plus we probably shouldn't do them at this time.

Comment on lines 17 to 18
INPUT_DIR = join(dirname(__file__), "input")
MSG_DIR = join(dirname(__file__), "messages")
Copy link
Member

Choose a reason for hiding this comment

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

I think it's safer.

I agree. Let's revert it here.

tests/test_pylint_runners.py Outdated Show resolved Hide resolved
tests/test_pylint_runners.py Outdated Show resolved Hide resolved
tests/test_regr.py Outdated Show resolved Hide resolved
tests/test_func.py Outdated Show resolved Hide resolved
Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com>
@DanielNoord DanielNoord requested a review from cdce8p April 14, 2022 16:27
@DanielNoord DanielNoord merged commit 8f872bf into pylint-dev:main Apr 14, 2022
@DanielNoord DanielNoord deleted the 3.7-1 branch April 14, 2022 17:38
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.

4 participants