Skip to content

ENH: Nullable integer/boolean/floating support in lib inferencing functions #40914

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 16 commits into from
May 5, 2021

Conversation

lithomas1
Copy link
Member

Precursor for #40687

@lithomas1 lithomas1 added Enhancement NA - MaskedArrays Related to pd.NA and nullable extension arrays Dtype Conversions Unexpected or buggy dtype conversions labels Apr 13, 2021
BooleanArray(
np.array([True, False], dtype="bool"), np.array([False, True])
),
),
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have tests for maybe_convert_bool already? can you integrate with those

Copy link
Member Author

Choose a reason for hiding this comment

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

No. maybe_convert_bool is only used by python parser.

@lithomas1 lithomas1 requested a review from jbrockmendel April 21, 2021 18:29
@lithomas1 lithomas1 requested a review from jreback April 21, 2021 23:50
@jreback jreback modified the milestone: 1.3 Apr 26, 2021
@lithomas1 lithomas1 requested a review from jreback April 27, 2021 02:22
@lithomas1 lithomas1 added this to the 1.3 milestone Apr 27, 2021
@@ -676,7 +676,7 @@ def _infer_types(self, values, na_values, try_num_bool=True):
if try_num_bool and is_object_dtype(values.dtype):
# exclude e.g DatetimeIndex here
try:
result = lib.maybe_convert_numeric(values, na_values, False)
result, _ = lib.maybe_convert_numeric(values, na_values, False)
Copy link
Member Author

Choose a reason for hiding this comment

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

This will be addressed in #40687, the main PR.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

can you fix the return type in this PR? its very odd now

@lithomas1
Copy link
Member Author

@jreback Not sure I follow here. I'm just following

hmm can we simply make the return value always a tuple, e.g. (values, mask) where mask could be None

so that would be tuple[np.ndarray, np.ndarray | None]. We are not returning the masked EA directly since it is an anti-pattern.

@lithomas1 lithomas1 requested a review from jreback May 4, 2021 17:22
@jreback jreback merged commit 4d73a34 into pandas-dev:master May 5, 2021
@jreback
Copy link
Contributor

jreback commented May 5, 2021

thanks @lithomas1

if you wouldn't mind doing a PR to add doc-strings to the maybe_* routines, the issue is the Returns is now not super obvious.

thanks.

@jorisvandenbossche
Copy link
Member

Thanks a lot @lithomas1 !

@lithomas1 lithomas1 deleted the lib-nullable-inference branch May 29, 2021 17:55
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
Dtype Conversions Unexpected or buggy dtype conversions Enhancement NA - MaskedArrays Related to pd.NA and nullable extension arrays
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants