BUG: prevent coercion to datetime64[ns] when a Series is initialized with both tz-naive and tz-aware #18361
Conversation
|
Hello @tmnhat2001! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on November 23, 2017 at 02:49 Hours UTC |
pandas/_libs/tslib.pyx
Outdated
| for i in range(n): | ||
| val = values[i] | ||
| if _checknull_with_nat(val): | ||
| iresult[i] = NPY_NAT | ||
| elif PyDateTime_Check(val): | ||
| if val.tzinfo is not None: | ||
| if tz_naive_or_tz_aware is None: |
There was a problem hiding this comment.
What is this accomplishing that ‘inferred_tz’ isn’t?
|
so where you are fixing is not the correct location, its a bit after the actual error, rather try here: a mixed datetime64 & datetime64 that is tz-aware are in the same bucket, rather this needs disambiguation, and ultimately should be 'mixed'. Note we have no tests for this routine, so that is the first thing we need. Pls add to |
|
my bad, should have checked out that routine |
|
np.datetime64 objects don't have timezone info right? I just want to make sure not to leave it out. |
|
no, no.datetime64 don’t support time zone (they print with a local time zone which is extra confusing) |
Codecov Report
@@ Coverage Diff @@
## master #18361 +/- ##
==========================================
- Coverage 91.35% 91.34% -0.02%
==========================================
Files 163 163
Lines 49691 49691
==========================================
- Hits 45397 45388 -9
- Misses 4294 4303 +9
Continue to review full report at Codecov.
|
doc/source/whatsnew/v0.22.0.txt
Outdated
| @@ -178,5 +178,5 @@ Other | |||
| ^^^^^ | |||
|
|
|||
| - Improved error message when attempting to use a Python keyword as an identifier in a numexpr query (:issue:`18221`) | |||
| - | |||
| - Bug in :func:`inference.pyx.infer_to_datetimelike_array` that coerces tz-naive and tz-aware values in the same array to datetime64[ns] (:issue:`16406`) | |||
There was a problem hiding this comment.
give a user perspective here (IOW don't reference internal functions).
pandas/core/series.py
Outdated
| @@ -3238,7 +3240,8 @@ def _try_cast(arr, take_fast_path): | |||
| else: | |||
| subarr = maybe_convert_platform(data) | |||
|
|
|||
| subarr = maybe_cast_to_datetime(subarr, dtype) | |||
| if try_cast_to_datetime: | |||
There was a problem hiding this comment.
sorry about that, i was trying out something similar to infer_datetimelike_array in that function before you told me to try in inference.pyx. I'll change it back to the original version.
| assert lib.infer_datetimelike_array(arr) == "date" | ||
|
|
||
| @pytest.mark.parametrize( | ||
| "data", |
There was a problem hiding this comment.
can you add strings, ints, float, and a python object
e.g.
class Foo():
pass;
Foo()
should all return mixed (as this is a date infered)
| @@ -949,3 +949,8 @@ def test_get_level_values_box(self): | |||
| index = MultiIndex(levels=levels, labels=labels) | |||
|
|
|||
| assert isinstance(index.get_level_values(0)[0], Timestamp) | |||
|
|
|||
| def test_tz_naive_and_tz_aware_mix(self): | |||
| s = Series([Timestamp('20130101'), | |||
There was a problem hiding this comment.
add the issue number as a comment
do a full comparision with assert_series_equal, IOW construct the expected
move this test to
test_constructors.py
|
@tmnhat2001 nice patch! keep em coming! |
|
thanks |
git diff upstream/master -u -- "*.py" | flake8 --diff