-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
BUG: infer_string not inferring string dtype when NA is first value #55655
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.
@lithomas1 was looking at this function recently. Might want to review as well
@@ -2665,6 +2665,8 @@ def maybe_convert_objects(ndarray[object] objects, | |||
else: | |||
seen.object_ = True | |||
break | |||
elif val is C_NA: | |||
seen.object_ = True |
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 think missing a break
after this? See it in all the other branches at least
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.
No this is missing on purpose. We don't know which dtype we have with NA and infer-string set, so have to do another pass
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.
Ah OK. Does continue
work here too? Assuming no difference with the current code setup, but continue would be more future-proof in case of a refactor, while also signaling intent
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.
Yeah sure
thanks @phofl. |
…dtype when NA is first value
…tring dtype when NA is first value) (#55666) Backport PR #55655: BUG: infer_string not inferring string dtype when NA is first value Co-authored-by: Patrick Hoefler <[email protected]>
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.