BUG: to_datetime with unit with Int64 #30241

AlexKirko · 2019-12-12T20:01:05Z

closes TypeError in to_datetime when passing Int64 column #30050
tests added 1 / passed 1
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Test output:

$ py.test pandas/tests/tools/test_datetime.py
============================= test session starts =============================
platform win32 -- Python 3.7.3, pytest-5.3.1, py-1.8.0, pluggy-0.13.0
rootdir: C:\git_contrib\pandas\pandas, inifile: setup.cfg
plugins: hypothesis-4.50.6, cov-2.8.1, forked-1.1.2, xdist-1.30.0
collected 1 item

pandas\tests\tools\test_datetime.py .                                    [100%]

============================== 1 passed in 0.03s ==============================

Notes: Hello. The fix introduces one additional type check if the Series passed to to_datetime stores anything except and IntegerArray. It does nothing else in this case. If we try to pass an IntegerArray, it pulls all the values that aren't NaN into an ndarray, converts that into datetime and adds NaT in the places where NaNs were. No precision is lost.

pandas/tests/tools/test_datetime.py

pandas/core/tools/datetimes.py

jreback · 2019-12-15T21:50:03Z

@AlexKirko the diff is super confusing because you are pulling on other commits, pls merge in master and repush

AlexKirko · 2019-12-16T08:56:52Z

@AlexKirko the diff is super confusing because you are pulling on other commits, pls merge in master and repush

@jreback Apologies for the mess. Still learning how to contribute properly. Merged master in.

AlexKirko · 2019-12-20T07:01:39Z

@jreback Conflicts in whatsnew removed. Should be ready for another review.

AlexKirko · 2019-12-24T08:32:56Z

@jreback Merged in master again to remove new conflicts in datetime.py

pandas/core/tools/datetimes.py

AlexKirko · 2019-12-29T11:52:04Z

Moved the nan-handling to tslib. Web_and_Docs PR test is broken for now (# 30527), but other than that, things look ready for a review.

@jreback , did you have something like this solution in mind?

AlexKirko · 2019-12-30T07:58:28Z

@jreback Merged in master to fix Webs_and_Docs PR test bug.

jreback

lgtm (yes this was the approach was looking for). some small comments, ping on green.

jreback · 2019-12-30T13:31:22Z

pandas/_libs/tslib.pyx

@@ -297,16 +297,32 @@ def format_array_from_datetime(ndarray[int64_t] values, object tz=None,


 def array_with_unit_to_datetime(ndarray values, object unit,
-                                str errors='coerce'):
+                                str errors='coerce', ndarray mask=None):


let's move this after values, and require it

pandas/_libs/tslib.pyx

pandas/core/tools/datetimes.py

jreback · 2019-12-30T13:33:24Z

pandas/core/tools/datetimes.py

+        # because it expects an ndarray argument
+        if isinstance(arg, IntegerArray):
+            # Explicitly pass NaT mask to array_with_unit_to_datetime
+            mask_na = arg.isna()


just call this mask

jreback · 2019-12-30T13:33:53Z

pandas/core/tools/datetimes.py

+            # Explicitly pass NaT mask to array_with_unit_to_datetime
+            mask_na = arg.isna()
+            arg = getattr(arg, "_ndarray_values", arg)
+            result, tz_parsed = tslib.array_with_unit_to_datetime(


move this outside the if/else and add mask=None for the non-IntegerArray case, then you can just call this once

AlexKirko · 2019-12-30T15:43:04Z

@jreback Addressed all the comments, please take a look.

pandas/_libs/tslib.pyx

TomAugspurger · 2019-12-30T17:05:07Z

pandas/core/tools/datetimes.py

+        if isinstance(arg, IntegerArray):
+            # Explicitly pass NaT mask to array_with_unit_to_datetime
+            mask = arg.isna()
+            arg = getattr(arg, "_ndarray_values", arg)


Why getattr instead of arg._ndarray_values?

Once we have IntegerArray.to_numpy, I think this could be something like arg.to_numpy("i8", fill_value=NPY_NAT).

yes would like to do this exactly (or a way to get data/mask publicly would be ok here as well). but what you are suggesting is nice.

pandas/tests/indexes/datetimes/test_tools.py

pep8speaks · 2019-12-30T17:41:50Z

Hello @AlexKirko! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-01-01 08:03:42 UTC

AlexKirko · 2019-12-30T18:16:05Z

@jreback @TomAugspurger Added to the test and removed unnecessary getattr. Awaiting the decision on where we should implement NaN-handling: in tslib or in datetimes. I'm for tslib (current solution) as we merely expand on the way NaNs are handled and that's, in my opinion, is better than splitting and re-merging arg in _convert_listlike_datetimes.

jreback

small comment, pls rebase and ping on green.

pandas/_libs/tslib.pyx

pandas/tests/indexes/datetimes/test_tools.py

Stop relying on inner implementations of IntegerArray and numpy arrays. Use public funcitons as much as possible.

AlexKirko · 2020-01-01T08:55:14Z

@jreback rebased to updated master, squashed tiny commits, made the needed changes and commented on green

TomAugspurger

Thanks @AlexKirko, this looks good. Merging later today, in case @jreback wants to have one more look.

TomAugspurger · 2020-01-02T20:38:44Z

Thanks @AlexKirko!

AlexKirko · 2020-01-03T06:14:23Z

@jreback @TomAugspurger Thank you! Learned a lot about the process: it should help a lot with the next PR.

jreback requested changes Dec 13, 2019

View reviewed changes

pandas/tests/tools/test_datetime.py Outdated Show resolved Hide resolved

pandas/core/tools/datetimes.py Outdated Show resolved Hide resolved

jreback added ExtensionArray Extending pandas with custom dtypes or arrays. Datetime Datetime data dtype labels Dec 13, 2019

AlexKirko requested a review from jreback December 15, 2019 06:33

jreback reviewed Dec 24, 2019

View reviewed changes

pandas/core/tools/datetimes.py Show resolved Hide resolved

jreback changed the title ~~Fix to datetime~~ BUG: to_datetime with unit with Int64 Dec 24, 2019

AlexKirko force-pushed the FIX-to-datetime branch 2 times, most recently from 34a0189 to 96c8ad8 Compare December 29, 2019 09:08

AlexKirko requested a review from jreback December 29, 2019 11:52

jreback requested changes Dec 30, 2019

View reviewed changes

jreback added this to the 1.0 milestone Dec 30, 2019

AlexKirko requested a review from jreback December 30, 2019 15:42

TomAugspurger reviewed Dec 30, 2019

View reviewed changes

jreback requested changes Jan 1, 2020

View reviewed changes

pandas/_libs/tslib.pyx Outdated Show resolved Hide resolved

pandas/tests/indexes/datetimes/test_tools.py Outdated Show resolved Hide resolved

AlexKirko force-pushed the FIX-to-datetime branch from 7634487 to 7fad79d Compare January 1, 2020 07:10

AlexKirko added 3 commits January 1, 2020 10:59

BUG: close pandas-dev#30050 - initial solution

65f5c41

CLN: switch to generic functions

f925f83

Stop relying on inner implementations of IntegerArray and numpy arrays. Use public funcitons as much as possible.

BUG: move nan-handling to tslib and clean up

620506d

AlexKirko force-pushed the FIX-to-datetime branch from 7fad79d to 620506d Compare January 1, 2020 08:03

AlexKirko requested a review from jreback January 1, 2020 08:55

TomAugspurger approved these changes Jan 2, 2020

View reviewed changes

TomAugspurger merged commit b804372 into pandas-dev:master Jan 2, 2020

AlexKirko deleted the FIX-to-datetime branch January 3, 2020 06:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: to_datetime with unit with Int64 #30241

BUG: to_datetime with unit with Int64 #30241

AlexKirko commented Dec 12, 2019

jreback commented Dec 15, 2019

AlexKirko commented Dec 16, 2019 •

edited

Loading

AlexKirko commented Dec 20, 2019

AlexKirko commented Dec 24, 2019

AlexKirko commented Dec 29, 2019 •

edited

Loading

AlexKirko commented Dec 30, 2019 •

edited

Loading

jreback left a comment

jreback Dec 30, 2019

jreback Dec 30, 2019

jreback Dec 30, 2019

AlexKirko commented Dec 30, 2019

TomAugspurger Dec 30, 2019

TomAugspurger Dec 30, 2019

jreback Jan 1, 2020

pep8speaks commented Dec 30, 2019 •

edited

Loading

AlexKirko commented Dec 30, 2019 •

edited

Loading

jreback left a comment

AlexKirko commented Jan 1, 2020

TomAugspurger left a comment

TomAugspurger commented Jan 2, 2020

AlexKirko commented Jan 3, 2020

BUG: to_datetime with unit with Int64 #30241

BUG: to_datetime with unit with Int64 #30241

Conversation

AlexKirko commented Dec 12, 2019

jreback commented Dec 15, 2019

AlexKirko commented Dec 16, 2019 • edited Loading

AlexKirko commented Dec 20, 2019

AlexKirko commented Dec 24, 2019

AlexKirko commented Dec 29, 2019 • edited Loading

AlexKirko commented Dec 30, 2019 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

jreback Dec 30, 2019

Choose a reason for hiding this comment

jreback Dec 30, 2019

Choose a reason for hiding this comment

jreback Dec 30, 2019

Choose a reason for hiding this comment

AlexKirko commented Dec 30, 2019

TomAugspurger Dec 30, 2019

Choose a reason for hiding this comment

TomAugspurger Dec 30, 2019

Choose a reason for hiding this comment

jreback Jan 1, 2020

Choose a reason for hiding this comment

pep8speaks commented Dec 30, 2019 • edited Loading

Comment last updated at 2020-01-01 08:03:42 UTC

AlexKirko commented Dec 30, 2019 • edited Loading

jreback left a comment

Choose a reason for hiding this comment

AlexKirko commented Jan 1, 2020

TomAugspurger left a comment

Choose a reason for hiding this comment

TomAugspurger commented Jan 2, 2020

AlexKirko commented Jan 3, 2020

AlexKirko commented Dec 16, 2019 •

edited

Loading

AlexKirko commented Dec 29, 2019 •

edited

Loading

AlexKirko commented Dec 30, 2019 •

edited

Loading

pep8speaks commented Dec 30, 2019 •

edited

Loading

AlexKirko commented Dec 30, 2019 •

edited

Loading