BUG: Series/Index results in datetime/timedelta incorrectly if inputs are all nan/nat like #13477

sinhrks · 2016-06-18T04:12:59Z

closes API: NaT/nan mixed input coerces to timedelta64 #13467
tests added / passed
passes git diff upstream/master | flake8 --diff

codecov-io · 2016-06-18T05:05:13Z

Current coverage is 84.33%

Merging #13477 into master will not change coverage

@@             master     #13477   diff @@
==========================================
  Files           138        138          
  Lines         51092      51092          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits          43087      43087          
  Misses         8005       8005          
  Partials          0          0

Powered by Codecov. Last updated by c2cc68d...a017a8c

jreback · 2016-06-18T14:37:29Z

pandas/src/inference.pyx

@@ -1,5 +1,6 @@
 import sys
 cimport util
+from tslib cimport is_timestamp


I think I might move this here as well (though may cause a circular import issue).

jreback · 2016-06-18T14:37:51Z

@sinhrks puzzled why this fixed things as all you did was move something

sinhrks · 2016-06-19T02:12:19Z

ATM, it doesn't affect to the behavior. However, I found some inconsistency when I adding the test. Updated in #13467.

jreback · 2016-06-19T18:48:21Z

pandas/src/inference.pyx


-    if util.is_datetime64_object(val) or val is NaT:
+        # same logic as is_null_datetimelike except for integer nat


if its the same logic, then let's just add a flag that includes/excludes it rather then replicating the logic
(or make another function sitting below it that does that).

@jreback The logic should be updated after a decision in #13467. Could you check?

jreback · 2016-06-24T09:57:32Z

ok as I commented on the other issue, I think all -> NaT/nan should be M8.

jreback · 2016-06-30T10:36:08Z

@sinhrks ready to merge? or is there still an open question on this

sinhrks · 2016-07-02T18:50:52Z

@jreback Updated based on #13467, and found that there is a test which expect np.timedelta('nat') is regarded as timedelta dtype (I think it's natural)

https://github.com/pydata/pandas/blob/master/pandas/tests/series/test_quantile.py#L42

How about skipping only np.nan and pd.NaT in infer_dtype? I mean:

[pd.NaT, np.datetime64('nat')] -> datetime
[pd.NaT, np.timedelta64('nat')] -> timedelta
[np.datetime64('nat'), np.timedelta64('nat')] -> mixed (object)

jreback · 2016-07-02T18:53:50Z

yeah that looks right

the problem is we can't know definitively the inferred type if we only have pd.NaT (or just np.nan for that matter)

so I suppose object is correct in those cases (though might break things)

sinhrks · 2016-07-02T19:14:20Z

Yeah. pd.NaT should be ragarded as datetime in all nan-like case, if not specified otherwise (no np.datetime or np.timedelta).

[pd.NaT, np.nan] -> datetime (it results in timedelta on current master, NG)
[np.nan, pd.NaT] -> datetime (OK on current master)
[pd.NaT, pd.NaT] -> datetime

sinhrks · 2016-07-02T21:55:52Z

Updated based on the above discussion.

jreback · 2016-07-06T21:54:56Z

pandas/src/inference.pyx

        if is_datetime64_array(values):
            return 'datetime64'
        elif is_timedelta_or_timedelta64_array(values):
            return 'timedelta'

-    elif util.is_integer_object(val):


hmm I thought this was needed maybe on py3? (the integer object check and then check for timedelta)

Though test has been passed, will revert it for safety.

jreback · 2016-07-06T21:58:21Z

pandas/src/inference.pyx

+
+
+cdef inline bint is_null_datetime64(v):
+    # determine if we have a null for a datetime (or integer versions)x,


do we need to export these (as in tslib.pxd)?

Not needed ATM because tslib doesn't use is_null_datetimelike.

jreback · 2016-07-06T21:59:02Z

@sinhrks looks really good. esp all of the added tests! just a couple of very minor questions. ping when ready.

jorisvandenbossche · 2016-07-06T22:50:07Z

pandas/tests/series/test_constructors.py

+        tm.assert_series_equal(Series([pd.NaT, pd.NaT]), exp)
+        tm.assert_series_equal(Series(np.array([pd.NaT, pd.NaT])), exp)
+
+        exp = Series(pd.DatetimeIndex([pd.NaT, pd.NaT]))


What is the difference with the exp above (exp = Series([pd.NaT, pd.NaT]))?

Not needed because of the bug fix. Removed.

sinhrks · 2016-07-07T15:04:01Z

Now all green.

sinhrks · 2016-07-07T15:04:46Z

pandas/src/inference.pyx

        if is_datetime64_array(values):
            return 'datetime64'
        elif is_timedelta_or_timedelta64_array(values):
            return 'timedelta'

+    elif is_timedelta(val):


This must be moved here. Otherwise, timedelta and object mixed data is regarded as "mixed-integer".

jreback · 2016-07-10T21:51:26Z

IIRC this was green. but rebase and ping.

sinhrks · 2016-07-11T05:42:17Z

@jreback @jorisvandenbossche rebased, and all green.

jorisvandenbossche · 2016-07-11T07:31:21Z

@sinhrks Thanks!

closes pandas-dev#13467

sinhrks added Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Clean labels Jun 18, 2016

sinhrks added this to the 0.18.2 milestone Jun 18, 2016

sinhrks force-pushed the nat_nan branch from 9e5f4a1 to c758ccc Compare June 18, 2016 04:21

jreback reviewed Jun 18, 2016
View reviewed changes

sinhrks force-pushed the nat_nan branch from c758ccc to 7ec8aca Compare June 19, 2016 01:57

sinhrks force-pushed the nat_nan branch 3 times, most recently from 32b86f9 to 03c016c Compare June 19, 2016 05:02

jreback reviewed Jun 19, 2016
View reviewed changes

sinhrks changed the title ~~CLN/TST: Add tests for nan/nat mixed input~~ API: Add tests for nan/nat mixed input Jun 24, 2016

sinhrks force-pushed the nat_nan branch from 03c016c to 5219fd2 Compare July 2, 2016 00:11

sinhrks force-pushed the nat_nan branch from 5219fd2 to 8a3702f Compare July 2, 2016 20:06

sinhrks changed the title ~~API: Add tests for nan/nat mixed input~~ BUG: Series/Index results in datetime/timedelta incorrectly if inputs are all nan/nat like Jul 2, 2016

sinhrks force-pushed the nat_nan branch from 8a3702f to 0682867 Compare July 2, 2016 20:31

sinhrks force-pushed the nat_nan branch 3 times, most recently from e539f34 to cd859a4 Compare July 5, 2016 19:28

jreback reviewed Jul 6, 2016
View reviewed changes

jorisvandenbossche reviewed Jul 6, 2016
View reviewed changes

sinhrks force-pushed the nat_nan branch from cd859a4 to 85b7648 Compare July 7, 2016 13:18

sinhrks added the Bug label Jul 7, 2016

sinhrks force-pushed the nat_nan branch from 85b7648 to 2a5d3ac Compare July 7, 2016 13:52

sinhrks reviewed Jul 7, 2016
View reviewed changes

sinhrks force-pushed the nat_nan branch 2 times, most recently from e6dc033 to 9dfda77 Compare July 10, 2016 21:40

CLN/TST: Add tests for nan/nat mixed input

a017a8c

sinhrks force-pushed the nat_nan branch from 9dfda77 to a017a8c Compare July 10, 2016 22:11

jorisvandenbossche merged commit 5605f99 into pandas-dev:master Jul 11, 2016

sinhrks deleted the nat_nan branch July 11, 2016 07:52

sinhrks mentioned this pull request Jul 15, 2016

API: Index doesn't results in PeriodIndex if Period contains NaT #13664

Closed

3 tasks

nateGeorge pushed a commit to nateGeorge/pandas that referenced this pull request Aug 15, 2016

CLN/TST: Add tests for nan/nat mixed input (pandas-dev#13477)

1f8cc7f

closes pandas-dev#13467

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Series/Index results in datetime/timedelta incorrectly if inputs are all nan/nat like #13477

BUG: Series/Index results in datetime/timedelta incorrectly if inputs are all nan/nat like #13477

sinhrks commented Jun 18, 2016

codecov-io commented Jun 18, 2016 •

edited

Loading

jreback Jun 18, 2016

jreback commented Jun 18, 2016

sinhrks commented Jun 19, 2016 •

edited

Loading

jreback Jun 19, 2016

sinhrks Jun 24, 2016

jreback commented Jun 24, 2016

jreback commented Jun 30, 2016

sinhrks commented Jul 2, 2016

jreback commented Jul 2, 2016

sinhrks commented Jul 2, 2016 •

edited

Loading

sinhrks commented Jul 2, 2016

jreback Jul 6, 2016

sinhrks Jul 7, 2016

jreback Jul 6, 2016

sinhrks Jul 7, 2016

jreback commented Jul 6, 2016

jorisvandenbossche Jul 6, 2016

sinhrks Jul 7, 2016

sinhrks commented Jul 7, 2016

sinhrks Jul 7, 2016

jreback commented Jul 10, 2016

sinhrks commented Jul 11, 2016

jorisvandenbossche commented Jul 11, 2016


		if util.is_datetime64_object(val) or val is NaT:
		# same logic as is_null_datetimelike except for integer nat



		cdef inline bint is_null_datetime64(v):
		# determine if we have a null for a datetime (or integer versions)x,

BUG: Series/Index results in datetime/timedelta incorrectly if inputs are all nan/nat like #13477

BUG: Series/Index results in datetime/timedelta incorrectly if inputs are all nan/nat like #13477

Conversation

sinhrks commented Jun 18, 2016

codecov-io commented Jun 18, 2016 • edited Loading

Current coverage is 84.33%

Choose a reason for hiding this comment

jreback commented Jun 18, 2016

sinhrks commented Jun 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Jun 24, 2016

jreback commented Jun 30, 2016

sinhrks commented Jul 2, 2016

jreback commented Jul 2, 2016

sinhrks commented Jul 2, 2016 • edited Loading

sinhrks commented Jul 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Jul 6, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sinhrks commented Jul 7, 2016

Choose a reason for hiding this comment

jreback commented Jul 10, 2016

sinhrks commented Jul 11, 2016

jorisvandenbossche commented Jul 11, 2016

codecov-io commented Jun 18, 2016 •

edited

Loading

sinhrks commented Jun 19, 2016 •

edited

Loading

sinhrks commented Jul 2, 2016 •

edited

Loading