Performance improvements for nunique method.

[lexual]

No description provided.

[jreback]

I would add the actual argument dropna to unique

[lexual]

@jreback I must be doing something idiotic, but when I add dropna keyword-arg to unique method in base.py, and a call to unique(dropna=dropna) in nunique, I get an error like below which I don't understand:

TypeError: unique() got an unexpected keyword argument 'dropna'

[jreback]

their is prob a test calling core/categorical/unique which needs this argument too

[lexual]

so a dropna parameter on Categorical's unique method doesn't make sense, as it appears that a Categorical objects can't have np.nan as one of it's levels.

[jreback]

just put **kwargs

it just needs to be able to accept it not do anything

[lexual]

I think DateTimeIndex needs it too.

[jreback]

DatetimeIndex call the one in base

[lexual]

@jreback Got an implementation of dropna method for you to have a look at ;)

[jreback]

I bet it hits the AttributeError every time. values are numpy arrays here, you have to do self.dropna()

[lexual]

I can confirm that the test_base.py executes all 4 paths (return statements) in that code, if that answers your query?

[lexual]

Nevermind, I think I understand now ;(

[jreback]

it just seems complicated. can you trace the paths? the ONLY path that has a unique for .values is Categorical. Maybe enumerate the paths and test for speed. A side issue, could add dropna method to Index as well (in core/base). Its actually pretty easy.

[sinhrks]

NaT will not be excluded by dropna, otherwise value_counts excludes NaT.

[lexual]

What use case is this line supporting? When would it get executed?

https://github.com/pydata/pandas/blob/master/pandas/core/base.py#L291

if hasattr(values,'unique'):
    return values.unique()

[jreback]

@lexual that causes a dispatch for Categorical as self.values are actually a Categorical (and NOT an ndarray).

[lexual]

Isn't a call of the unique method on a Categorical going to call this code, and not run the above code at all?

https://github.com/pydata/pandas/blob/master/pandas/core/categorical.py#L879

def unique(self):
    return self.levels

[jreback]

no, its a Series that's going to call this, which has a categorical as its values.

(Sparse types also have this feature, where the .values is NOT an ndarray).

In [1]:  Series(pd.Categorical([1,2,3,4]))
Out[1]: 
0    1
1    2
2    3
3    4
dtype: category
Levels (4, int64): [1 < 2 < 3 < 4]

In [2]:  Series(pd.Categorical([1,2,3,4])).unique()
Out[2]: Int64Index([1, 2, 3, 4], dtype='int64')

In [3]:  Series(pd.Categorical([1,2,3,4])).values
Out[3]: 
 1
 2
 3
 4
Levels (4, int64): [1 < 2 < 3 < 4]

In [4]: type(Series(pd.Categorical([1,2,3,4])).values)
Out[4]: pandas.core.categorical.Categorical

[lexual]

@sinhrks I'm sorry I don't follow your comment.

With this patch, DatetimeIndex's unique appears to work correctly:

i = pd.DatetimeIndex(['2014-01-01', pd.NaT])
assert len(i.unique(dropna=True)) == 1
assert len(i.unique(dropna=False)) == 2

[sinhrks]

@lexual Thanks to confirm. I've misunderstood.

[lexual]

@jreback OK, have reverted to checking hasattr instead of try/except (was thinking the whole, better to ask forgiveness, than permission thing), but this probably makes the code a little clearer.

Yes all 4 paths do get exercised.

I would definitely say adding dropna to index would be a good idea. It would speed up that path a lot, as it's currently very slow. And it would mean we could get rid of the 2nd path, as it would be handled by the first one.

Very late here, so I'm offline until tomorrow.

[jreback]

why don't u add a dropna as we'll then (their is an issue outstanding about it somewhere)

but let's do this in a separate issue

the index ops have a hasnan property now so this should be straightforward

[lexual]

OK, now have dropna on Index, and this being used by nunique.

[jreback]

@lexual going to need a couple of vbenches for this. I think their exists ones for unique so add near there. You can do a shorter one (say 3000 elements and a longer one, 100k elements). try to create so they are somewhat stable run-to-run (e.g. don't create them randomly), but just string together sub-seqequences.

[jreback]

@lexual also going to needs some tests for dropna for DatetimeIndex/PeriodIndex. Note that these already have some machinery in place, so you may need to redefine it in (core/base/DatetimeOpsMixin); see hasnans as well

[lexual]

@jreback might be a little while before I could find time to make these extra amends:

• Never used vbench before, so need to figure how to install/run/write before I even get started.
• Crazy busy for probably next week at least.

Cheers,

[jreback]

@lexual no problem. Ok let's leave this in place.

@sinhrks do you want to take for dropna into base? (test and impl)?

[lexual]

@jreback added vbench for nunique. Probably need someone to look at, new to this tool

[jreback]

should just be self[~isnull(self)]

[lexual]

tests fail with that change:

raise NotImplementedError("isnull is not defined for MultiIndex")

[jreback]

hmm ok

going to need a test for each index type for dropna (except Int64) of course

[jreback]

an you post your vbench results when have them.

[jreback]

@lexual can you rebase, how's this coming?

[lexual]

Need to figure out how to run vbench, and how to share results.
Also figure out how to get tests written for DatetimeIndex/PeriodIndex.

[jreback]

closing as stale, but issue is now at #9354