Skip to content

Fix result type of DataFrame.align #577

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

Merged
merged 3 commits into from
Mar 16, 2023
Merged

Fix result type of DataFrame.align #577

merged 3 commits into from
Mar 16, 2023

Conversation

Azureblade3808
Copy link
Contributor

The result type of DataFrame.align should be a tuple instead of DataFrame.

@@ -558,7 +559,7 @@ class DataFrame(NDFrame, OpsMixin):
limit: int | None = ...,
fill_axis: Axis = ...,
broadcast_axis: Axis | None = ...,
) -> DataFrame: ...
) -> tuple[DataFrame, _SeriesOrFrameT]: ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you can use pandas._typing.NDFreamT instead of creating a new TypeVar.

Can you please also add a quick test?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree to use NDFrameT, because that will give the precise type of Series or DataFrame based on other

Tests should test using other as a DataFrame and other as a Series

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dr-Irv Dr-Irv merged commit 522b3b6 into pandas-dev:main Mar 16, 2023
twoertwein pushed a commit to twoertwein/pandas-stubs that referenced this pull request Apr 1, 2023
* Fix result type of `DataFrame.align`

* Use `NDFrameT` instead of newly added `_SeriesOrFrameT`

* Add tests for `DataFrame.align`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants