ENH: Deprecate non-keyword arguments for Index.set_names. #41551
Conversation
pandas/core/indexes/base.py
Outdated
| @@ -1526,6 +1527,7 @@ def _set_names(self, values, level=None) -> None: | |||
|
|
|||
| names = property(fset=_set_names, fget=_get_names) | |||
|
|
|||
| @deprecate_nonkeyword_arguments(version="2.0", allowed_args=["self", "names"]) | |||
There was a problem hiding this comment.
Can you put version=None? I know I'd originally put version="2.0", but then other reviewers expressed a preference for not specifying the exact version.
There was a problem hiding this comment.
Added in my latest commit.
I think these changes also apply to the PR I opened for drop_duplicates (#41500)
Would you like me to make these changes there too?
There was a problem hiding this comment.
Yes please, thanks! Your help with these is much appreciated
| with tm.assert_produces_warning(FutureWarning, match=msg): | ||
| idx.set_names(["kind", "year"], None) |
There was a problem hiding this comment.
Can you also check the result? Again, I'm aware that I didn't do this in the example PR I opened, but then other reviewers suggested it - there's an example in #41511 . Same for the Series test
There was a problem hiding this comment.
No problem, added in the latest commit.
simonjayhawkins
added
Deprecate
| idx = MultiIndex.from_product([["python", "cobra"], [2018, 2019]]) | ||
|
|
||
| msg = ( | ||
| "In a future version of pandas all arguments of Index.set_names " |
There was a problem hiding this comment.
same as in the other PR, you'll probably need to define this for MultiIndex too (and remove @final) so that the warning shows MultiIndex.set_names
There was a problem hiding this comment.
Made this change in fc525c8.
There was a problem hiding this comment.
There's a typing failure in multi.py (because we don't yet have overloads here - but those'll come once these deprecation warnings are in) - for now, could you just silence it? So
diff --git a/pandas/core/indexes/multi.py b/pandas/core/indexes/multi.py
index 3c070889c1..269a73cd3c 100644
--- a/pandas/core/indexes/multi.py
+++ b/pandas/core/indexes/multi.py
@@ -3594,7 +3594,8 @@ class MultiIndex(Index):
"""
names = self._maybe_match_names(other)
if self.names != names:
- return self.rename(names)
+ # Incompatible return value type (got "Optional[MultiIndex]", expected "MultiIndex")
+ return self.rename(names) # type: ignore[return-value]
return self
def _maybe_match_names(self, other):
|
can you merge master |
…cate-nonkeyword-args-set_index
| @@ -3585,7 +3584,9 @@ def _get_reconciled_name_object(self, other) -> MultiIndex: | |||
| """ | |||
| names = self._maybe_match_names(other) | |||
| if self.names != names: | |||
| return self.rename(names) | |||
| # Incompatible return value type (got "Optional[MultiIndex]", expected | |||
| # "MultiIndex") | |||
There was a problem hiding this comment.
try doing assert isinstance(self, MultiIndex) here
There was a problem hiding this comment.
That would work, but for things that will be fixed in the future (e.g. this one once all the overloads are in, which will come after positional arguments are deprecated) I think we've generally been ignoring the error
There was a problem hiding this comment.
kk, then we can just merge this.
inplace#41485