GH1264 Version 2.0 cleanup by loicdiridollou · Pull Request #1265 · pandas-dev/pandas-stubs (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation11 Commits2 Checks13 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
- Closes Follow-up version 2.0 #1264
- Tests added: Please use
assert_type()to assert the type of any return value
| def union(self, other: list[HashableT] | Index, sort=...) -> Index: ... | |
|---|---|
| def intersection(self, other: list[S1] | Self, sort: bool = ...) -> Self: ... | |
| def union( | |
| self, other: list[HashableT] | Index, sort: bool | None = ... |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| self, other: list[HashableT] | Index, sort: bool | None = ... |
|---|---|
| self, other: list[HashableT] | Self, sort: bool | None = ... |
Might as well be consistent with these different operations
| ) -> TimedeltaSeries: ... |
|---|
| def cumprod( |
| self, |
| axis: AxisIndex | None = ..., |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove None
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the default argument is None, wouldn't that cause issues?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean, I was looking at the 2.3.0 code which still has None as default but on main it is now 0 (I would not see anyone use None by default and otherwise it will make it a good time to fix it).
Comment on lines 321 to 330
| pd.RangeIndex(0, 10).union(["a", "b", "c"], sort=True), |
|---|
| Union[pd.Index, "pd.Index[int]", pd.RangeIndex], |
| ), |
| pd.Index, |
| ) |
| check( |
| assert_type( |
| pd.RangeIndex(0, 10).union(["a", "b", "c"], sort=False), |
| Union[pd.Index, "pd.Index[int]", pd.RangeIndex], |
| ), |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem right, and I think you'll need some specific overloads here for RangeIndex (and Sequence[int] for that matter). If RangeIndex has a set operation with either a Sequence[int], Series[int], Index[int] or RangeIndex, the result is Union[RangeIndex, Index[int]]. With any other type, it is just Index
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed that was the wrong copy paste, this is fixed.
| t = pd.Series([1, 2], index="ab") # type: ignore[call-overload] # pyright: ignore[reportCallIssue, reportArgumentType] |
|---|
| def test_timedelta_index_cumsum() -> None: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def test_timedelta_index_cumsum() -> None: |
|---|
| def test_timedelta_invalid_cumprod() -> None: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 participants
