BUG: GH11206 where pd.isnull did not consider numpy NaT null
State: closed

closes #11206


Labels:
name: Datetime
description: Datetime data dtype
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
lgtm.

can you run a perf check on the null benchmarks. just confirm nothing has substantially changed.

Updated. Timing for `frame_methods.frame_isnull.time_frame_isnull` (where all data is non-`nan` `float`s) increased by less than 5%.
Added a benchmark where lots of the data is various types of null.

```
BENCHMARK                                    BEFORE      AFTER     FACTOR
...ame_isnull_has_null.time_frame_isnull    38.18ms   138.88ms   3.63779915x
```

I also changed the code slightly so that the logic is further short-circuited short-circuited. 

the benchmark u are showing is 3x increase?

Yes.That's the with lots of nulls. I will dig a little to see if that can be avoided. 

these routines are hit _everywhere_, so i am for special casing certain things. IOW, that is why `isnullobj` exists. But it may make sense to have 2 versions of `_checknull_with_nat`, because they are called on different array kinds. E.g if you have an int array then it obviously doesn't need to check for `None,NaN,NaT` at all. most of these cases are already handled, but IIRC, this is hotspotted. So just want to try to nail that down.

Updated.

udpated

ok. I'm traveling and will get to this next week. 

@kawochen can you update (move whatsnew to 0.17.1)

Will do (still on vacation)

> On Oct 9, 2015, at 8:35 AM, Jeff Reback notifications@github.com wrote:
> 
> @kawochen can you update (move whatsnew to 0.17.1)
> 
> —
> Reply to this email directly or view it on GitHub.

updated

can u rebase and run a perf check

Saw some horrifying perf hit (30X).  I will look at the compiled code later.

ok gr8!

so using `timeit` and this code

```
from pandas import *
import pandas as pd
import numpy as np

np.random.seed(1234)
sample = np.array([NaT, np.nan, None, np.datetime64('NaT'),
    np.timedelta64('NaT'), 0, 1, 2.0, '', 'abcd'])
data = np.random.choice(sample, (1000, 1000))
df = DataFrame(data)

%timeit isnull(df)
```

the perf hit is 4X.  But if I that put into an `asv` benchmark, it shows 24X. I seem to be having problems with `asv` lately -- not sure what's wrong.

are you using `profile=True` in the cython modules?

The perf hit is different for numpy 1.10 vs 1.9 (and asv probably uses the newest version)

@kawochen pushing, but if you finish in the next few days, pls ping

@pv thanks! :smile: 
@jreback 
branch
`
[ 32.58%] ··· Running ...s.frame_isnull_obj.time_frame_isnull    46.07ms
`
master
`
[ 82.58%] ··· Running ...s.frame_isnull_obj.time_frame_isnull    38.91ms
`

`

Replacing wouldn't work in all cases, because sometimes you don't want `timedelta64` to be accepted, e.g. `datetime_to_datetime64()`, so it would become some sort of manual inlining for `_checknull_with_nat` to deal with those cases (which might not be a bad thing, depending on how often this is needed).

But I see that `tslib.pyx` needs some cleaning, as code like `if val is np_NaT or val.view('i8') == iNaT` is everywhere (`np_NaT` is not a singleton, so the first half of the expression is almost certainly `False`, and the `val.view('i8')` is slower than `get_datetime64_val` (especially given the regression in numpy 1.10, but that will be fixed soon).

So maybe when others or I clean up this module, we can see how to organize or deal with all the lower level scalar check null functions

@kawochen thanks!

always nice PR's from you! keep it up!