API: change unique to return Index

[sinhrks]

• closes Index.unique() should always return an Index object of the same type #13395, closes Call unique() on a timezone aware datetime series returns non timezone aware result #13565
• tests added / passed
• passes git diff upstream/master | flake8 --diff
• whatsnew entry

[jreback]

what about for. multi index?

[sinhrks]

@jreback changed to return MI. Adding explicit tests.

mi = pd.MultiIndex.from_arrays([[1, 2, 1, 2], [1, 1, 1, 2]])
mi 
# MultiIndex(levels=[[1, 2], [1, 2]],
#            labels=[[0, 1, 0, 1], [0, 0, 0, 1]])

# on current master
mi.unique()
# array([(1, 1), (2, 1), (2, 2)], dtype=object)

# after this PR
pd.MultiIndex.from_arrays([[1, 2, 1, 2], [1, 1, 1, 2]]).unique()
# MultiIndex(levels=[[1, 2], [1, 2]],
#            labels=[[0, 1, 1], [0, 0, 1]])

[jreback]

I don't think u need special case for CI
it defaults to same categories and ordered

[sinhrks]

We have to update categories because Categorical.unique reorders categories if ordered=False. This can be simplified if Categorical.unique doesn't change categories.

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

[jorisvandenbossche]

I am not sure anymore why we decided to reorder the categories, but that does not sounds logical to me. @sinhrks do you recall the reasoning?

The values in the index should be in order of appearance, but that does not mean the categories attribute should be changed

[jreback]

hmm, I think we did have some discussion about this, but I don't remember,
cc @JanSchulz

[jankatins]

The PR is #10508, and the why seems to be #10505. So @sinhrks needs to answer this :-)

Somehow this looks a lot like a user facing API (unique -> why does it change the categories at all? IMO it should just return all categories even íf some are unused: you also don't change maxint if it isn't in an integer series) was changed to fix a internal user which gave a undesired result (probably groupby shouldn't return empty groups?).

[sinhrks]

Thanks for the reminder, I'd clean forgotten it.

it was mainly done to keep category dtype, and not sure whether keeping categories order breaks anything. let me check...

[jorisvandenbossche]

I don't think the actual decision how to sort/not sort the return value of Categorical.unique mattered for the bug fix. But, in retrospect, I think we took the wrong choice there. IMO, categoricals should just follow everything else: unique values in order of appearance, and leave the dtype/categories alone.

[codecov-io]

Current coverage is 85.27% (diff: 100%)

Merging #13979 into master will increase coverage by <.01%

@@             master     #13979   diff @@
==========================================
  Files           139        139          
  Lines         50502      50511     +9   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43063      43071     +8   
- Misses         7439       7440     +1   
  Partials          0          0          

Powered by Codecov. Last update be61825...28a2781

[jreback]

@sinhrks ok as per discussion

Index -> should return Index
Series -> match return types now (no API change):

In [1]: Series([1,2,3]).unique()
Out[1]: array([1, 2, 3])

In [2]: Series([1,2,3.5]).unique()
Out[2]: array([ 1. ,  2. ,  3.5])

In [3]: Series(['foo','bar','baz']).unique()
Out[3]: array(['foo', 'bar', 'baz'], dtype=object)

In [4]: Series(['foo','bar','baz'],dtype='category').unique()
Out[4]: 
[foo, bar, baz]
Categories (3, object): [foo, bar, baz]

In [6]: Series(pd.date_range('20130101',periods=3)).unique()
Out[6]: 
array(['2013-01-01T00:00:00.000000000', '2013-01-02T00:00:00.000000000',
       '2013-01-03T00:00:00.000000000'], dtype='datetime64[ns]')

#### this discussed in other issue should be object array of Timestamps (and is API change)
In [7]: Series(pd.date_range('20130101',periods=3,tz='US/Eastern')).unique()
Out[7]: 
array(['2013-01-01T05:00:00.000000000', '2013-01-02T05:00:00.000000000',
       '2013-01-03T05:00:00.000000000'], dtype='datetime64[ns]')

# to be honest would change this too to an object array of ``Timedelta``s, but leave for now
In [8]: Series(pd.timedelta_range('1 day',periods=3)).unique()
Out[8]: array([ 86400000000000, 172800000000000, 259200000000000], dtype='timedelta64[ns]')

# good
In [9]: Series(pd.period_range('2013-1',periods=3,freq='M')).unique()
Out[9]: 
array([Period('2013-01', 'M'), Period('2013-02', 'M'),
       Period('2013-03', 'M')], dtype=object)

[sinhrks]

yep, will update.

[jorisvandenbossche]

I would put this also in the api changes section, as it was not a bug (it was a deliberate decision I think, but we changed our mind)

[jorisvandenbossche]

Looks good to me!
Small comment on the whatsnew note, but that can also be done after merge.

[sinhrks]

Moved Series.unique description to API change.

Just in case, categorical Series.unique returns Categorical rather than ndarray now (not changed by this PR)

pd.Series([1, 2], dtype='category').unique()
# [1, 2]
# Categories (2, int64): [1, 2]

[jreback]

Index.unique() now returns unique values as an Index of the appropriate dtype

[jorisvandenbossche]

@sinhrks There is a small linting error:

pandas/indexes/base.py:18:1: F401 'pandas.types.generic.ABCCategoricalIndex' imported but unused

Can be merged apart from that.

[jorisvandenbossche]

@sinhrks Thanks a lot!