-
-
Notifications
You must be signed in to change notification settings - Fork 25.2k
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
MNT clean futurewarning for 1.0 | _deprecate_positional_args #20002
Conversation
I simply silenced mypy errors by adding mypy errors
|
You're not removing the actual helper, right? Wouldn't it be good to do that, so we can make sure we got them all? |
The helper is still there, it is required until 1.1 for this function: scikit-learn/sklearn/inspection/_plot/partial_dependence.py Lines 823 to 825 in a9cc0ed
Result of
|
I pushed a commit to bump the default version value to "1.1 (renaming of 0.26)" in the private @thomasjpfan the mypy linter now complains with:
I could probably just silence it but do you understand what is causing this? I checked and |
My sense is that decorators + mypy can give strange results. In this case, adding |
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.
+1 for merging this PR then. Thanks @thomasjpfan for the mypy investigation / fix.
This will make our tracebacks and profiler report simpler and easier to understand. |
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
Remove
@_deprecate_positional_args
for 1.0 (part of #19335).These decorators have been added in 0.23 (see #15005).
Now that positional arguments are not allowed, we can safely reorder parameters when appropriate (maybe better in a separate issue though).
Commands used