API: add "level=" argument to MultiIndex.unique()
State: closed

- [x] closes #17896
- [x] tests added / passed
- [x] passes `git diff master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Labels:
name: API Design
name: MultiIndex

Comments:
From #17881 , @jreback 's [comments](https://github.com/pandas-dev/pandas/issues/17896#issuecomment-336975055):

> I am not sure unique is the right word here.

I'm not a fan either...

> .get_level_values(level, used=False), though I am not sure I like this either.

... but I think this would be pretty confusing ("used" as in "this label appears in the index" vs. "used" as "this label is used multiple times")
Alternatives:
- ``MultiIndex.get_level_values(idx, drop_duplicates=False)``
- ``MultiIndex.get_level_values(idx, duplicated=True)``
how about ``remove_unused_levels=False``, so then ``remove_unused_levels`` is obvious
> how about remove_unused_levels=False, so then remove_unused_levels is obvious

you mean "levels" or "labels"? Anyway, not following you: ``get_level_values()`` always removes unused labels... the particularity of this is that (used) labels only appear once.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=h1) Report
> Merging [#17897](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/5bf7f9a4ff6968fd444fa7098f8dc95586591994?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/17897/graphs/tree.svg?width=650&src=pr&token=eZ4WkYLtcO&height=150)](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #17897      +/-   ##
==========================================
- Coverage   91.23%   91.22%   -0.02%     
==========================================
  Files         163      163              
  Lines       50105    50107       +2     
==========================================
- Hits        45715    45708       -7     
- Misses       4390     4399       +9
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.03% <100%> (ø)` | :arrow_up: |
| #single | `40.31% <75%> (-0.06%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/indexes/multi.py](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9tdWx0aS5weQ==) | `96.4% <100%> (ø)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.75% <0%> (-0.1%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=footer). Last update [5bf7f9a...8a69543](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=h1) Report
> Merging [#17897](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/a172ff9dd32d7f6ba077c82b6cf6a31dcc7f2b45?src=pr&el=desc) will **decrease** coverage by `0.04%`.
> The diff coverage is `95.23%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/17897/graphs/tree.svg?height=150&width=650&src=pr&token=eZ4WkYLtcO)](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #17897      +/-   ##
==========================================
- Coverage   91.38%   91.34%   -0.05%     
==========================================
  Files         164      164              
  Lines       49797    49812      +15     
==========================================
- Hits        45508    45501       -7     
- Misses       4289     4311      +22
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.14% <95.23%> (-0.03%)` | :arrow_down: |
| #single | `39.55% <61.9%> (-0.06%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `96.42% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/multi.py](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9tdWx0aS5weQ==) | `96.4% <100%> (+0.02%)` | :arrow_up: |
| [pandas/core/indexes/category.py](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9jYXRlZ29yeS5weQ==) | `97.2% <75%> (-0.26%)` | :arrow_down: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `63.44% <0%> (-1.82%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.8% <0%> (-0.1%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=footer). Last update [a172ff9...feb65ed](https://codecov.io/gh/pandas-dev/pandas/pull/17897?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Answering @jorisvandenbossche [from here](https://github.com/pandas-dev/pandas/issues/17896#issuecomment-337002044):

> I agree it would be nice to have a clean way to get those unique values, but IMO it does not belong in get_level_values. That method returns the actual values of the Index level, with a length equal to the length of the Index, and IMO we should stick to that contract. Having such a keyword would completely alter the return type of this method.
> 
> (not directly a good idea for alternative though)

I'm open to alternatives, but let me summarize my rationale for working on ``get_level_values()``:
- the difference between ``mi.levels`` ("implementation detail") and ``mi.get_level_values()`` (the "effective" content of the index level) has always confused users, but at least it finally reduces to remembering the difference between the two. In this sense, the set of "effective" unique values appearing in the level clearly pertains to the second.
- I've always considered the ``get_level_values()`` name itself misleading - it reminds me of something like ``value_counts()``, not of something like ``mi.levels[idx]`` or ``df[col]``, which get entire levels/columns. If I had to redesign the API from scratch, I would probably call ``get_level_values()`` precisely the version with ``unique=True`` (and ``get_level()`` the version with ``unique=False``)
- in general, I would like to avoid adding new methods to an API which already has too many
- (whatever choice we make for the API, the work will be done by the internal ``_get_level_values``, but this is an implementation detail)

All this said, an alternative which I would find elegant enough would be to add an argument ``level`` to ``MultiIndex.unique()``. It could even accept a list, which could be useful. The only disadvantage I see would be that it would deviate from the signature of ``Index.unique()``, but after all this already happens for ``MultiIndex.drop(level=)``.
Two additional elements in favor of ``get_level_values()``:
- we might want to add an argument ``sort=`` which switches between the order the elements appear in ``mi.levels[level]`` and the order the elements appear in ``mi`` (the implementation is trivial). This would be specific to ``MultiIndex``es (sorting might be undefined for a generic ``Index``) and hence a bit out of place in ``MultiIndex.unique()``. Vice-versa, it would be trivial (albeit of limited utility) to support it also in conjunction with ``unique=False``.
- the standard way as of now to (inefficiently) obtain the same result is precisely ``mi.get_level_values(level).unique()``
OK, trying to clarify my rationale for objecting adding it to `get_level_values`.

So indeed we have to distinguish the `levels` (the "unique labels" for each level) and the `get_level_values` (the "effective values" inside one level, as a full Index). And I agree that the naming is not ideal (additionally with `labels` not being the labels but the integer codes ..).

>  In this sense, the set of "effective" unique values appearing in the level clearly pertains to the second.

You could indeed see it that way (*unique* "effective values"), but I would rather see it as the *used* "unique labels". If you look at it like that, it clearly pertains to the first ..

In that sense, suppose we had a method like `.get_level(i)` (which would be equivalent to `.levels[i]`), I think it would have belonged in that method: something like `.get_level(i, remove_unused=True)`. 
But of course, we don't have that method, and I agree we should avoid adding new methods ..

> All this said, an alternative which I would find elegant enough would be to add an argument level to MultiIndex.unique()

This actually sounds like a nice alternative.


> the standard way as of now to (inefficiently) obtain the same result is precisely mi.get_level_values(level).unique()

or `mi.remove_unused_levels().levels[i]` (which will also be more efficient)
> or mi.remove_unused_levels().levels[i] (which will also be more efficient)

Good point. Still, it's recent and hence probably not very well known. And in principle _less_ efficient for short indexes with multiple long levels.

Anyway, I'm OK with changing this PR to ``MultiIndex.unique(level=)``: @jreback what do you think?
> Anyway, I'm OK with changing this PR to MultiIndex.unique(level=): @jreback what do you think?

seems reasonable.

alternatively, ``.get_levels()`` is available, sure I don't like expanding the API but it is explicit.
> alternatively, .get_levels() is available, sure I don't like expanding the API but it is explicit.

I'm afraid it would be confusing (I would love to swap its name with ``get_level_values()``, but it's too late), so I went for ``unique(level=)``.
The first ``xfail`` is unrelated to my patch to the code, and just reflects the fact that I replaced ``tm.assert_numpy_array_equal`` with ``tm.assert_index_equal``, as suggested by @jorisvandenbossche 
can you rebase and will take a look.
OK, ping @jreback 
The second commit changes the behavior of ``Index(., dtype='category').unique()`` to _not_ change the order of categories (to the order of appearance). I know this is inconsistent with both ``Categorical.unique()`` and ``Series(., dtype='category').unique()``, but I would rather change the latter two, because I don't see the point of ``unique()`` changing the categories at all.

If instead the current behavior is preferred, that commit can be dropped, and I will just need to exclude the ``category`` case from [this test](https://github.com/pandas-dev/pandas/pull/17897/files#diff-9ae0a94014431c93e23da48433811c49R334) - because ``drop_duplicates`` and ``unique`` currently give different results.
> but I would rather change the latter two, because I don't see the point of unique() changing the categories at all.

how did this come up? why do you think that the consistency should diverge here?
> how did this come up?

My new test compares ``drop_duplicates`` with ``unique``, which differ for ``category``, precisely because the categories change. But my argument is clearly not "let's save the test", it is "why should an operation which clearly doesn't change the set of distinct values affect the ``categories``"?

> why do you think that the consistency should diverge here?

I'm not in favor of breaking consistency (between ``Index``, ``Categorical`` and ``Series``), I would rather change them all.
@jorisvandenbossche 
The behavior of ``Categorical.unique()`` and friends (``Series``, ``Index``) is probably worth discussing separately and fixing all together, so I created #18291, and changed the problematic test in this PR so it temporarily skips any ``CategoricalIndex``.
( @jreback : ping)
Will take a look at this tomorrow!
needs a rebase
ping
Thanks!