ENH: reset_index on a MultiIndex with duplicate levels raises a ValueError
State: closed

- [x ] closes #44410
- [x ] tests added / passed
- [x ] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit) for how to run them
- [x ] whatsnew entry


Labels:
name: MultiIndex

Comments:
We should discuss this first, @mroeschke and I agreed that we should only add a note to the docs
@phofl 

> We should discuss this first, @mroeschke and I agreed that we should only add a note to the docs

By all means! This arose due to  @rhshadrach's efforts to break my #44267. I found that a trivial and sensible change to frame.py fixed the problem, and also fixed #44410. I know that you decided to document rather than fix. But since the bug was a regression, and the fix seems simple, I suggest to accept my fix. It will also be a prerequisite for my #44267.
Imo this change is neither trivial nor simple. Accidentially ending up with duplictated column names is not desireable. This is a real PITA in indexing among others. For example:

```
df = pd.DataFrame([[1, 2, 3]], columns=["a", "a", "b"])
```

``df["b"]`` returns a Series while ``df["a"]`` returns a DataFrame.
IMO this is more of an api change instead of a bugfix. Also I think you can make a point that this should be changed inside of insert, not on a higher level.

> Accidentially ending up with duplictated column names is not desireable.
> IMO this is more of an api change instead of a bugfix.

Agreed on both counts. For #44267, it is the case that the user called the function with duplicate columns, the internals of groupby moved them into the index, and we want to restore them as columns. For this, it would suffice to have an internal version of reset_index where we can override allow_duplicates, i.e. this pattern:

```
def foo(x):
    return _foo(x, hidden_arg=False)

def _foo(x, hidden_arg=False):
    pass
```

where #44267 could then call `_foo(x, hidden_arg=True)` (where `hidden_arg` is `allow_duplicates`).

> Also I think you can make a point that this should be changed inside of insert, not on a higher level.

I'm not sure what this means (in other words, what should be changed inside of insert?). In any case, for #44267, I don't think we should be reimplementing reset_index via calling insert directly (to be clear, I'm not suggesting you're saying that).
What I meant with respect to insert:

If we want to respect the flag, we should change the keyword ``allow_duplicates``  to None or lib.NO_DEFAULT and figure out the correct default inside of insert instead of the caller.

But I would prefer not changing the public behavior, so I like your idea with a private implementation better.
There is a whole chapter on this: https://pandas.pydata.org/pandas-docs/stable/user_guide/duplicates.html.
It implies that the Pandas policy is to default to _allowing_ duplicates:

> Both Series and DataFrame disallow duplicate labels by calling `.set_flags(allows_duplicate_labels=False)`. (_the default is to allow them_).

It seems to me that if you have a legal Series (with duplicates), then it is OK for `reset_index()` to translate  it to a legal DataFrame (with duplicates).
My change just takes `allow_duplicates` from the frame's own` allows_duplicate_labels` flag.
But it is not for me to say. If you want reset_index() to have an allow_duplicates flag (over-riding the frame's flag) then let me know. 
I can't see anything gained by hiding it in a private method. SInce #44410 is easily resolved then why not let it be resolved?
> SInce #44410 is easily resolved then why not let it be resolved?

I personally am not a fan of this resolution. My understanding is that `flags.allow_duplicate_labels` will not allow duplicates in both the index and the columns. In using pandas, I very often find use in having duplicates in the index, but never want them in the columns. In other words, I want `reset_index` to throw an error when the result would have duplicate columns but I am not able to set `flags.allow_duplicate_labels` to be True.

In any case, this an API change that will need to have a FutureWarning before it can be implemented in 2.0 if we go this route.
Hello @johnzangwill! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:









There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2022-01-28 22:44:47 UTC
I have added `allow_duplicates` to DataFrame.reset_index() and Series.reset_index(). I have reverted and added to the tests accordingly, and changed the whatsnew from bug to enhancement.
The defaults are both `False`, so the current (pre-PR) behaviour is unchanged: duplicates are not created, regardless of the frame flag.
#44410 still raises `ValueError`. But adding `allow_duplicates=None` or `allow_duplicates=True` makes reset_index() work as the OP proposed.
> we would have to expose this argument to virtually all functions I don't think its worth it.

I must be missing something!

@rhshadrach has argued that that the default should be to raise. This behaviour was introducted in 1.3.0. So I have left that as the default and added an optional argument to restore the old functionality, namely to allow duplicates.

Since the new default (with the argument missing) is the same as the current functionality, then I don't see that anything has to be changed. Certainly all tests pass.

Both GroupBy.size() and my proposed DataFrameGroupBy.value_counts() need this option to cope with pathological as_index=False cases.

I have basically the same expectation as @rhshadrach. Dupicates in an Index are ok, but I do not want them in the columns.

> @rhshadrach has argued that that the default should be to raise. This behaviour was introducted in 1.3.0.

What are you referring to here? ``reset_index`` did raise before 1.3 too.


@phofl 

> What are you referring to here? `reset_index` did raise before 1.3 too.

Maybe I was mistaken about that. In any case, my change does not change that behavior.
thinking again ok to expose this options here
> Something like `"use_flag"`?

Changed and added test.
@jreback, @rhshadrach 
It seems that Pandas has two different things:

1. The `allows_duplicate_labels` flag of DataFrame. This defaults to **True** and allows external datasets to be loaded with duplicate row and column names. It can be set to False but, as I said elsewhere, many things break.
2. The `allow_duplicates` argument of DataFrame.insert(). This is to temporarily allow additional columns to be inserted into an existing frame with duplicated column names. This defaults to **False**, regardless of the frame flag. Its purpose is to stop duplicated columns being created, which is usually a sign of an upsteam error.

This PR is just to allow reset_index() to have access to the frame.inset() allow_duplicates argument. It needs no more support for the flag than DataFrame.insert() does.

In any case, the argument only works if the flag is True. If the flag is False and the argument True, then DataFrame.insert() raises, something which is included in my tests.
> flags.allows_duplicate_labels = False seems broken due to inplace handling all over the place (Yes, I will make an Issue).

AFAICT `allows_duplicate_labels` is basically half-implemented (along with `__finalize__` constitutes a large portion of our xfailed tests).  We should either fully support it or deprecate/remove it.  I have held off on arguing for this bc until now it hasn't been causing problems elsewhere, but it appears to be causing problems here.

> appears to be causing problems here.

Not causing me problems (apart from confusing everybody!).
I am just pointing out that it is slightly broken (Issue #44958). And its problems are at least partly to be blamed on the following pattern, which is used in quite a few places:
```
if not inplace:
    obj = self.copy()
    obj.do_something(inplace=True)
```
which may or may not be consistent with a future plan to phase-out inplace...

@jreback All green, apart from CI being broken again. Hopefully, the comments above explain why this is a harmless and useful addition. I would like it in 1.4 if possible, because I still want to use it to add duplicate handling to DataFrameGroupBy.value_counts (#44992). Thanks!