Skip to content
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

Fix the metadata propagation in Dataframe.std #52924

Merged
merged 4 commits into from
May 1, 2023

Conversation

jiawei-zhang-a
Copy link
Contributor

@jiawei-zhang-a jiawei-zhang-a commented Apr 26, 2023

@jiawei-zhang-a jiawei-zhang-a changed the title Fix the metadata propagation both in Dataframe.std Fix the metadata propagation in Dataframe.std Apr 26, 2023
@mroeschke
Copy link
Member

Looks like there's typing issue which indicates this needs fixed at a lower level

@mroeschke mroeschke added the metadata _metadata, .attrs label Apr 26, 2023
@jiawei-zhang-a
Copy link
Contributor Author

Yeah it seems so, i will figure it out

@jiawei-zhang-a
Copy link
Contributor Author

Fix the typing problem by forcing the only propagation on Series

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

thanks for working on this! this is close

also, might be good to add a whatsnew note to 2.1.0?

Comment on lines 11075 to 11076
else:
return result
Copy link
Member

Choose a reason for hiding this comment

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

this should be unreachable, and looks like a case where we humans looking at the code can infer things which the type checker can't

I'd suggest

result = cast(Series, super().std(...)
return result.__finalize_(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you so much, Mr Gorelli! I appreciate your help so much, I will submit this:)

@MarcoGorelli
Copy link
Member

changes look good, if you add a whatsnew note to 2.1.0.rst we can get this in

@jiawei-zhang-a
Copy link
Contributor Author

Sure! I will add it now, Sorry I forget to add

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

@mroeschke mroeschke added this to the 2.1 milestone May 1, 2023
@mroeschke mroeschke merged commit 000c3ed into pandas-dev:main May 1, 2023
33 of 34 checks passed
@mroeschke
Copy link
Member

Thanks @jiawei-zhang-a

NumanIjaz pushed a commit to NumanIjaz/pandas that referenced this pull request May 1, 2023
* Fix the metadata propagation both in Dataframe.std

* Fix the typing problem by forcing propogation only on Series

* From judge statement  to cast

* Add whatsnew
topper-123 pushed a commit to topper-123/pandas that referenced this pull request May 7, 2023
* Fix the metadata propagation both in Dataframe.std

* Fix the typing problem by forcing propogation only on Series

* From judge statement  to cast

* Add whatsnew
Rylie-W pushed a commit to Rylie-W/pandas that referenced this pull request May 19, 2023
* Fix the metadata propagation both in Dataframe.std

* Fix the typing problem by forcing propogation only on Series

* From judge statement  to cast

* Add whatsnew
Daquisu pushed a commit to Daquisu/pandas that referenced this pull request Jul 8, 2023
* Fix the metadata propagation both in Dataframe.std

* Fix the typing problem by forcing propogation only on Series

* From judge statement  to cast

* Add whatsnew
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
metadata _metadata, .attrs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants