-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
Unexpected keyword argument #48914
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
Unexpected keyword argument #48914
Conversation
d009049
to
b0237ed
Compare
Signed-off-by: Soumik Dutta <[email protected]>
Signed-off-by: Soumik Dutta <[email protected]>
Signed-off-by: Soumik Dutta <[email protected]>
Signed-off-by: Soumik Dutta <[email protected]>
|
it's supported with |
Signed-off-by: Soumik Dutta <[email protected]>
name: Hashable | None = None, | ||
copy: bool | None = None, | ||
dtype=None, |
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.
Thanks for your PR
I don't think this is the right fix, as these arguments (name
, copy
, ...) get passed to NumericIndex
, rather than to _constructor
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.
Yes, I looked into the code. Classes ExtensionIndex
, MultiIndex
and NumericIndex
inherits from the base class Index
.
And
MultiIndex
andRangeIndex
(which inherits fromNumericIndex
)
is overriding the _constructor
method.
So should I include the function arguments in the _constructor
method of those classes too?
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.
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.
wait, this isn't resolved - I think the issue is that pylint can't tell that @cache_readonly
makes _constructor
act like a property and that it doesn't take arguments
I'll have another look later
of MultiIndex and RangeIndex Signed-off-by: Soumik Dutta <[email protected]>
name: Hashable | None = None, | ||
copy: bool | None = None, | ||
dtype=None, |
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.
wait, this isn't resolved - I think the issue is that pylint can't tell that @cache_readonly
makes _constructor
act like a property and that it doesn't take arguments
I'll have another look later
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.
Thanks @shalearkane for your PR
I've had a closer look, and I don't think we should enable this check
cache_readonly
acts like a property, but pylint
isn't aware of this and so it thinks that the arguments that are passed to whetever function the property returns are being passed to the property itself
mypy
already checks this, and the way pandas gets mypy to recognise that cache_readonly
is a property is
pandas/pandas/_libs/properties.pyi
Line 16 in 7440fe2
cache_readonly = property |
So, I'll add this to the list of checks we probably can't turn off
Closing for now then
This fixed unexpected-keyword-argument warnings from pylint on one file.
The other warnings produced were already expected to be caught in tests.