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

change the type annotation error heuristic #2583

Merged

Conversation

temyurchenko
Copy link
Contributor

@temyurchenko temyurchenko commented Sep 24, 2024

The previous one depended on the message from "typed_ast", which is
not used anymore.

Instead, we check if there is a "# type:" substring in the source line
of the exception. This can yield some false positives, but probably
rarely.

Description

Partially addresses pylint-dev/pylint#3757

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.24%. Comparing base (c8e8831) to head (7266e22).
Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2583      +/-   ##
==========================================
+ Coverage   93.10%   93.24%   +0.13%     
==========================================
  Files          93       93              
  Lines       11065    11067       +2     
==========================================
+ Hits        10302    10319      +17     
+ Misses        763      748      -15     
Flag Coverage Δ
linux 93.12% <100.00%> (+0.13%) ⬆️
pypy 93.24% <100.00%> (+0.13%) ⬆️
windows 93.22% <100.00%> (+0.13%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
astroid/builder.py 95.11% <100.00%> (+0.91%) ⬆️

... and 5 files with indirect coverage changes

DanielNoord
DanielNoord previously approved these changes Sep 25, 2024
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.

Thank you for the dive into the internal, much appreciated. Do you think it's possible to start testing this @temyurchenko ? I expected the coverage to go up but it wasn't tested before which explain that the dead code using a dead library is still there.

@temyurchenko
Copy link
Contributor Author

Thank you for the dive into the internal, much appreciated. Do you think it's possible to start testing this @temyurchenko ? I expected the coverage to go up but it wasn't tested before which explain that the dead code using a dead library is still there.

I'll try to add a test

The previous one depended on the message from "typed_ast", which is
   not used anymore.

Instead, we check if there is a "# type:" substring in the source line
   of the exception. This can yield some false positives, but probably
   rarely.
@temyurchenko
Copy link
Contributor Author

Thank you for the dive into the internal, much appreciated. Do you think it's possible to start testing this @temyurchenko ? I expected the coverage to go up but it wasn't tested before which explain that the dead code using a dead library is still there.

I've added a test and unblocked another one. It seems to have fixed pylint-dev/pylint#3757 and possibly pylint-dev/pylint#3556.

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.

Amazing !!

@Pierre-Sassoulas Pierre-Sassoulas added this to the 3.3.5 milestone Sep 26, 2024
@Pierre-Sassoulas Pierre-Sassoulas merged commit 62c5bad into pylint-dev:main Sep 26, 2024
21 checks passed
github-actions bot pushed a commit that referenced this pull request Sep 26, 2024
The previous one depended on the message from "typed_ast", which is
   not used anymore.

Instead, we check if there is a "# type:" substring in the source line
   of the exception. This can yield some false positives, but probably
   rarely.

(cherry picked from commit 62c5bad)
Pierre-Sassoulas pushed a commit that referenced this pull request Sep 26, 2024
The previous one depended on the message from "typed_ast", which is
   not used anymore.

Instead, we check if there is a "# type:" substring in the source line
   of the exception. This can yield some false positives, but probably
   rarely.

(cherry picked from commit 62c5bad)

Co-authored-by: temyurchenko <44875844+temyurchenko@users.noreply.github.com>
@temyurchenko temyurchenko deleted the fix-type-anno-heuristic branch September 26, 2024 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants