-
-
Notifications
You must be signed in to change notification settings - Fork 146
Improve results of stubtest #1286
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
Conversation
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.
Looks good to me, but I'm a bit unsure about the deprecated arguments.
Thanks @twoertwein for the review, I was debating myself about the deprecated args, they appear in the stubtest since the code is still there. |
pandas-stubs/core/series.pyi
Outdated
@@ -1132,6 +1135,7 @@ class Series(IndexOpsMixin[S1], NDFrame): | |||
freq: DateOffset | timedelta | _str | None = ..., | |||
axis: Axis = ..., | |||
fill_value: Scalar | NAType | None = ..., | |||
suffix: _str | None = ..., # has no effect at the moment |
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.
Probably should split into 2 overloads, based on the docs. One is where periods
is int
and there is no suffix
argument, and the other where periods
is Sequence[int]
and then suffix
is allowed.
I don't understand what you mean by "has no effect at the moment"
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.
If you read the source code it is not used at all.
https://github.com/pandas-dev/pandas/blob/4f952b79f91f4f9d4d86b46d34e78dc657bfd4a2/pandas/core/generic.py#L10122-L10130
Maybe I missed something in the code but not sure.
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 created pandas-dev/pandas#61955
so let's delete it from the stubs
I prefer to not have deprecated arguments in the stubs, because we want people to stop using them. Maybe there is a way to tell |
When it comes to stubtest I don't think there is a way for it to filter since the errors that we get is that the arguments in the stubs are misaligned with the runtime version (it is impossible for stubtest to know what is deprecated in pandas). |
pandas-stubs/core/series.pyi
Outdated
@@ -1132,6 +1135,7 @@ class Series(IndexOpsMixin[S1], NDFrame): | |||
freq: DateOffset | timedelta | _str | None = ..., | |||
axis: Axis = ..., | |||
fill_value: Scalar | NAType | None = ..., | |||
suffix: _str | None = ..., # has no effect at the moment |
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 created pandas-dev/pandas#61955
so let's delete it from the stubs
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 @loicdiridollou
Clean up some old pandas 2.2 code that was removed, improved the results of the stubtest.
assert_type()
to assert the type of any return value