-
-
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
Fix FP for unexpected-keyword-arg
with ambiguous constructors
#9785
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9785 +/- ##
==========================================
- Coverage 95.81% 95.79% -0.02%
==========================================
Files 174 174
Lines 18860 18873 +13
==========================================
+ Hits 18070 18079 +9
- Misses 790 794 +4
|
This comment has been minimized.
This comment has been minimized.
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 think for now this is fine. Indeed, at some point we might want to refactor this.
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, just a nit.
519b997
(cherry picked from commit dd20b30)
Type of Changes
Description
Closes #9672
I'm not sure we should always pile more special cases into safe_infer() instead of deferring some of this to the call sites to determine whether they care about all the inferred results or not, but it's a start.
We probably need to open a top-level issue for auditing whether all checks should be safe_infer()'ing or infer_all()'ing and doing something fancier. Maybe ask for community help?
For example, in this case, we could actually look at the keyword arguments of each inferred class and find out whether at least one of them has the keyword in question instead of just giving up completely when we find ambiguity.