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

[kawochen]

closes #11206

[jreback]

lgtm.

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

[kawochen]

Updated. Timing for frame_methods.frame_isnull.time_frame_isnull (where all data is non-nan floats) 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.

[jreback]

the benchmark u are showing is 3x increase?

[kawochen]

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

[jreback]

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.

[kawochen]

Updated.

[kawochen]

I forgot to remove this. I will remove in the next update.

[jreback]

what are you fixing here?

[kawochen]

you mean the _check_all_nulls? Only object arrays hit this path so I thought it'd be safe to have the most generic version of checknull here?

For the dtype thing, I forgot to remove it after some experiment, but will remove it in the next update.

[jreback]

yep that is fine

[kawochen]

udpated

[jreback]

let's put this in 0.17.1

[jreback]

pls move to 0.17.1

[kawochen]

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

[jreback]

@kawochen can you update (move whatsnew to 0.17.1)

[kawochen]

Will do (still on vacation)

On Oct 9, 2015, at 8:35 AM, Jeff Reback [email protected] wrote:

@kawochen can you update (move whatsnew to 0.17.1)

—
Reply to this email directly or view it on GitHub.

[kawochen]

updated

[jreback]

can u rebase and run a perf check

[kawochen]

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

[jreback]

ok gr8!

[kawochen]

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.

[jreback]

are you using profile=True in the cython modules?

[pv]

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

[jreback]

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

[kawochen]

@pv thanks! 😄
@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

`

[jreback]

maybe blow away _checknull_with_nat(and replace with _check_all_nulls and see if any timings are affected?

[kawochen]

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

[jreback]

@kawochen thanks!

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