Skip to content

BUG: fix convert_dtypes to handle empty df #40402

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

owenlamont
Copy link
Contributor

@owenlamont owenlamont commented Mar 12, 2021

Added a conditional to check a DataFrame had at least one column and if not return the original (empty) DataFrame if so.

Added a conditional to check a DataFrame had at least one column and if
not return the original (empty) DataFrame if so.
@owenlamont owenlamont marked this pull request as draft March 12, 2021 16:31
Added unit test and added reference to whatsnew
@owenlamont owenlamont marked this pull request as ready for review March 12, 2021 17:07
Copy link
Member

@jorisvandenbossche jorisvandenbossche 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 the PR!
Just a few small comments

Moved a frame test incorrectly placed in the
series hierarchy. Fixed some naming in bug
documentation. Changed convert_dtypes to
return a copy of apparently empty dataframes
@owenlamont
Copy link
Contributor Author

Thanks @jorisvandenbossche

All good call outs (particularly the test in the wrong place - I should've noticed that). I hope it looks better now.

@simonjayhawkins simonjayhawkins added Bug NA - MaskedArrays Related to pd.NA and nullable extension arrays labels Mar 14, 2021
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Looks really good - two minor requests

@jreback jreback added this to the 1.3 milestone Mar 14, 2021
Changed test to use tm.assert_equal and documented
error more explicitly
@owenlamont owenlamont requested a review from rhshadrach March 15, 2021 12:34
@owenlamont
Copy link
Contributor Author

Thanks for the feedback @rhshadrach I'm still not sure I've got the assert written the way you'd like - didn't know whether to compare the DataFrame to itself or to a new empty DataFrame. Please let me know if you'd like anything else updated.

Replaced tm.assert_equals with tm.assert_frame_equals
@jorisvandenbossche jorisvandenbossche changed the title BUG 40393 convert_dtypes to handle empty df BUG: fix convert_dtypes to handle empty df Mar 15, 2021
@jorisvandenbossche jorisvandenbossche merged commit 8a8df02 into pandas-dev:master Mar 15, 2021
@jorisvandenbossche
Copy link
Member

Thanks a lot @owenlamont for your first contribution!

@owenlamont
Copy link
Contributor Author

Thank you! I'm honoured to contribute to such a foundational part of the PyData ecosystem. First of many hopefully.

JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug NA - MaskedArrays Related to pd.NA and nullable extension arrays
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: convert_dtypes throws a ValueError when called on an empty DataFrame
5 participants