-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 regression tests. #9071
Add regression tests. #9071
Conversation
Failing CI tests should be fixed when pylint-dev/astroid#2307 is live. |
e01b501
to
b2afb38
Compare
fc99872
to
8ed99d6
Compare
Willte require astroid 2.15.8 when backporting, this is going to be easier to do manually. |
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.
Primer is non trivial to review, need to understand if it's change in the primed repo or due to astroid upgrade.
Mixture of astroid upgrade and π€·ββοΈ. The homeassistant/music21/pandas changes are same as in #9065. The others aren't. I have faith that we're making things less flaky overall, but I wouldn't say we've defeated flakiness (pylint-dev/astroid#2273 is outstanding). |
Oh, did you mean just this branch's upgrade from 3.0 beta to 3.0 final? |
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.
Thank you for the insight @jacobtylerwalls. I think the fix to #8998 will help us to make this review easier in the future by separating bleeding edge and upgrade.
I'm worried about pandas in particular (lots of new no-members
FP). They do not seems to be due to changes on the bleeding edge repo we use (https://github.com/pandas-dev/pandas/blame/61d2056e4251b8d1c387f2577f0407062b9ab903/pandas/core/resample.py#L387 was modified 7 months ago and just appeared). I don't see anything obvious that we changed between the last alpha of astroid and now that could be the cause though.
Actually this is due to the change from 3.0b to 3.0 final. You can reproduce with: python3.11 -m pylint tests/.pylint_primer_tests/pandas-dev/pandas/pandas/core/resample.py --disable=all --enable=no-member Before pylint-dev/astroid@89dfb485:
Then pylint-dev/astroid@89dfb485 (astroid 3.0 beta) removes 3 messages:
Then astroid 3.0 final reverts, giving the original 11 messages again:
The reason the messages went away in pylint-dev/astroid@89dfb485 is because inference got worse, and no-member gets shy when things are uninferable: 3.0 beta: (Pdb) node.expr
<Attribute._selected_obj l.1520 at 0x106b61d90>
(Pdb) node.expr.inferred()
[Uninferable] versus 3.0 final (Pdb) node.expr
<Attribute._selected_obj l.1520 at 0x107619e10>
(Pdb) node.expr.inferred()
[<BoundMethod _selected_obj of pandas.core.groupby.groupby.BaseGroupBy at 0x4817398224] So I think we're good here. |
I guess going forward we should be careful about backporting things that involve inference changes. |
It also seems |
Type of Changes
Description
Refs #9069
Refs pylint-dev/astroid#2305