-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add compatibility with astroid inference cache changes #8872
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 compatibility with astroid inference cache changes #8872
Conversation
273fb38
to
077e37d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. You can release if you want, you should have all the rights to do it afaik @jacobtylerwalls :)
I don't have PyPI access. |
Sorry for the abbreviated response :-) Happy to start doing alpha releases. I don't mind doing this atomic update first because there are other changes already in astroid main to .arguments that might need updates also. |
This comment has been minimized.
This comment has been minimized.
You don't need pypi access to release, creating a release from a tag on github will launch a release pipeline that will in turn publish on pypi. See https://pylint.readthedocs.io/en/stable/development_guide/contributor_guide/release.html |
This comment has been minimized.
This comment has been minimized.
@@ -41,7 +41,7 @@ dependencies = [ | |||
# Also upgrade requirements_test_min.txt. | |||
# Pinned to dev of second minor update to allow editable installs and fix primer issues, | |||
# see https://github.com/pylint-dev/astroid/issues/1341 | |||
"astroid>=3.0.0a8,<=3.1.0-dev0", | |||
"astroid @ git+https://github.com/pylint-dev/astroid.git@cf8763a2b8e897ec7c8389906f3cb13714300cd2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can then immediately go up to 3.0.0a9 next, but I wanted to do baby steps first in case more changes are needed.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #8872 +/- ##
==========================================
- Coverage 95.88% 95.74% -0.14%
==========================================
Files 173 173
Lines 18552 18557 +5
==========================================
- Hits 17789 17768 -21
- Misses 763 789 +26
|
This comment has been minimized.
This comment has been minimized.
# properties from parent classes. Given how much we cache inference | ||
# results, mutating instance_attrs can become a real mess. Filter | ||
# them out here until the root cause is solved. | ||
# https://github.com/pylint-dev/astroid/issues/2273 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a release blocker?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt it, since the current situation is better (more deterministic).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The primer results are looking a lot better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's with the deprecated-typing-alias
messages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the junk from #8790 is finally going away!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, I made a stab at the root cause in astroid and made a bit of progress, but there is still a way to go before mergeable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last nit
for more information, see https://pre-commit.ci
Thanks for the review! Excited to see the shorter primer run times and fewer messages. |
Type of Changes
Description
Adds compatibility with recent astroid changes discussed at pylint-dev/astroid#2257 (comment).