ENH: Add nullable keyword to read_sql #50048
Conversation
…ts_nullable_boolean # Conflicts: # pandas/_libs/lib.pyx
phofl
changed the title
|
I think makes sense and like your vision on the interaction with Cython. Is there any performance difference here moving away from the |
|
Also discussed this with @mroeschke in another pr, he leaned more towards keeping maybe_convert_objects as is. Maybe a bit faster? The previous implementation was running through the same function that we are calling directly now. Definitively no slowdown |
I think in a future PR it might make sense to do this; I didn't realized this was the only location where this is used. I guess the only slight downside is run time importing the Arrays in the Cython code? |
|
Thanks @phofl |
|
|
||
| if dtype is None: | ||
| if arr.dtype == np.dtype("O"): | ||
| # i.e. maybe_convert_objects didn't convert | ||
| arr = maybe_infer_to_datetimelike(arr) | ||
| if use_nullable_dtypes and arr.dtype == np.dtype("O"): | ||
| arr = StringDtype().construct_array_type()._from_sequence(arr) |
There was a problem hiding this comment.
this seems weird to me. why are we casting potentially non-strings to strings?
| arr = IntegerArray(arr, np.zeros(arr.shape, dtype=np.bool_)) | ||
| elif is_bool_dtype(arr.dtype): | ||
| arr = BooleanArray(arr, np.zeros(arr.shape, dtype=np.bool_)) | ||
| elif is_float_dtype(arr.dtype): |
There was a problem hiding this comment.
could re-use pd.array for L1011-L1016?
| dtype=None, | ||
| coerce_float=coerce_float, | ||
| use_nullable_dtypes=use_nullable_dtypes, | ||
| ) |
There was a problem hiding this comment.
looks like this has gotten some Arrow-specific logic added here in the interim. can we move the backend-specific stuff from core.internals here?
Sits on top of #50047
Functionality wise, this should work now.
More broadly, I am not sure that this is the best approach we could take here. Since the
convert_to_nullable_typeinlib.maybe_convert_objectsis not used right now except here, we could also make this strict and return the appropriate Array from the Cython code, not only when nulls are present. This would avoid the re-cast in the non-cython code part.