-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: replace coerces incorrect dtype #12780
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ | |
_ensure_int32, _ensure_int64, | ||
_NS_DTYPE, _TD_DTYPE, _INT64_DTYPE, | ||
_POSSIBLY_CAST_DTYPES) | ||
from .dtypes import ExtensionDtype | ||
from .dtypes import ExtensionDtype, DatetimeTZDtype, PeriodDtype | ||
from .generic import ABCDatetimeIndex, ABCPeriodIndex, ABCSeries | ||
from .missing import isnull, notnull | ||
from .inference import is_list_like | ||
|
@@ -310,8 +310,17 @@ def _maybe_promote(dtype, fill_value=np.nan): | |
return dtype, fill_value | ||
|
||
|
||
def _infer_dtype_from_scalar(val): | ||
""" interpret the dtype from a scalar """ | ||
def _infer_dtype_from_scalar(val, pandas_dtype=False): | ||
""" | ||
interpret the dtype from a scalar | ||
|
||
Parameters | ||
---------- | ||
pandas_dtype : bool, default False | ||
whether to infer dtype including pandas extension types. | ||
If False, scalar belongs to pandas extension types is inferred as | ||
object | ||
""" | ||
|
||
dtype = np.object_ | ||
|
||
|
@@ -334,13 +343,20 @@ def _infer_dtype_from_scalar(val): | |
|
||
dtype = np.object_ | ||
|
||
elif isinstance(val, (np.datetime64, | ||
datetime)) and getattr(val, 'tzinfo', None) is None: | ||
val = lib.Timestamp(val).value | ||
dtype = np.dtype('M8[ns]') | ||
elif isinstance(val, (np.datetime64, datetime)): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess category should raise NotImplmentedError? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As |
||
val = tslib.Timestamp(val) | ||
if val is tslib.NaT or val.tz is None: | ||
dtype = np.dtype('M8[ns]') | ||
else: | ||
if pandas_dtype: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e.g. here, is this a back-compat issue? |
||
dtype = DatetimeTZDtype(unit='ns', tz=val.tz) | ||
else: | ||
# return datetimetz as object | ||
return np.object_, val | ||
val = val.value | ||
|
||
elif isinstance(val, (np.timedelta64, timedelta)): | ||
val = lib.Timedelta(val).value | ||
val = tslib.Timedelta(val).value | ||
dtype = np.dtype('m8[ns]') | ||
|
||
elif is_bool(val): | ||
|
@@ -361,6 +377,11 @@ def _infer_dtype_from_scalar(val): | |
elif is_complex(val): | ||
dtype = np.complex_ | ||
|
||
elif pandas_dtype: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this parameter necessary, why wouldn't we always want to infer a pandas dtype if its there (e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not needed for the PR itself. The same func is required for #14145. |
||
if lib.is_period(val): | ||
dtype = PeriodDtype(freq=val.freq) | ||
val = val.ordinal | ||
|
||
return dtype, val | ||
|
||
|
||
|
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.
is this for another issue?
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, issued #15183. Let me fix it in a separate pr.