GH1139 Series.rename inplace #1140
Conversation
pandas-stubs/core/series.pyi
Outdated
| def rename( # pyright: ignore[reportOverlappingOverload] | ||
| self, | ||
| index: Renamer | Hashable | None = ..., | ||
| index: Hashable = ..., | ||
| *, | ||
| axis: Axis | None = ..., | ||
| copy: bool = ..., | ||
| inplace: Literal[True], | ||
| level: Level | None = ..., | ||
| errors: IgnoreRaise = ..., | ||
| ) -> Self: ... | ||
| @overload | ||
| def rename( # type: ignore[overload-cannot-match] | ||
| self, | ||
| index: Renamer | None = ..., |
There was a problem hiding this comment.
reverse the order of these two, and then make the one with Hashable be index: Hashable so that it has to be specified. That will change the ignore-related comments as well.
| @@ -1137,7 +1138,6 @@ def test_types_set_flags() -> None: | |||
|
|
|||
| def test_types_getitem() -> None: | |||
| s = pd.Series({"key": [0, 1, 2, 3]}) | |||
| key: list[int] = s["key"] | |||
There was a problem hiding this comment.
I think this was supposed to be a test. Should be
check(assert_type(s["key"], Any), list)
tests/test_series.py
Outdated
| check( | ||
| assert_type(pd.Series([1, 2, 3]).rename("A", inplace=True), "pd.Series[int]"), | ||
| pd.Series, | ||
| ) |
There was a problem hiding this comment.
You should add a test for specifying inplace=True with index=None being explicit, as well as a test for specifying inplace=True with index being a dict.
There was a problem hiding this comment.
The last issue is that when you have a lambda then it will match to Hashable and not Renamer so that is still an issue.
Feel free to offer feedback and release without this one, it is fine to wait till we make it right.
|
@loicdiridollou let me know when you think you'll get to this. I'd like to do a new release soon, and want to know if I should wait for this change or not. |
|
I was planning to take care of it in the evening but if you need to make a
release feel free to go ahead !3;943 that.
…On Tue, Mar 4, 2025 at 2:58 PM Irv Lustig ***@***.***> wrote:
@loicdiridollou <https://github.com/loicdiridollou> let me know when you
think you'll get to this. I'd like to do a new release soon, and want to
know if I should wait for this change or not.
—
Reply to this email directly, view it on GitHub
<#1140 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMQNSQCSSP6NGEOAKRHH2T32SYAXNAVCNFSM6AAAAABYFNRBHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMOJYG43DKMZVGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
[image: Dr-Irv]*Dr-Irv* left a comment (pandas-dev/pandas-stubs#1140)
<#1140 (comment)>
@loicdiridollou <https://github.com/loicdiridollou> let me know when you
think you'll get to this. I'd like to do a new release soon, and want to
know if I should wait for this change or not.
—
Reply to this email directly, view it on GitHub
<#1140 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMQNSQCSSP6NGEOAKRHH2T32SYAXNAVCNFSM6AAAAABYFNRBHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMOJYG43DKMZVGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
loicdiridollou
force-pushed
the
gh1139_rename
branch
from
73a1f14 to
2a112e6
Compare
loicdiridollou
force-pushed
the
gh1139_rename
branch
from
2a112e6 to
470ccf4
Compare
assert_type()to assert the type of any return valueAdded some test framework migration as this was how I originally found the issue with
rename