API: re-allow duplicate index level names

[jorisvandenbossche]

One possible solution for #19029

WIP (need to clean up tests and possibly re-add some ones that have been removed in #18882)

[codecov]

Codecov Report

Merging #21423 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #21423      +/-   ##
==========================================
+ Coverage    91.9%    91.9%   +<.01%     
==========================================
  Files         154      154              
  Lines       49558    49560       +2     
==========================================
+ Hits        45545    45548       +3     
+ Misses       4013     4012       -1

Flag	Coverage Δ
#multiple	90.27% <100%> (ø)	⬆️
#single	42.02% <20%> (-0.01%)	⬇️

Impacted Files	Coverage Δ
pandas/core/reshape/reshape.py	99.78% <100%> (ø)	⬆️
pandas/core/indexes/multi.py	94.96% <100%> (+0.07%)	⬆️
pandas/core/indexes/interval.py	93.13% <0%> (-0.04%)	⬇️
pandas/core/frame.py	97.19% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44c5460...fde8231. Read the comment docs.

[TomAugspurger]

+1 in general.

Will want a semi-detailed release note, potentially laying out future plans with respect to duplicate names?

[jorisvandenbossche]

Will want a semi-detailed release note, potentially laying out future plans with respect to duplicate names?

Until we don't exactly know what those future plans are, I would rather keep it limited (only noting that duplicated level names are allowed again?)

[jorisvandenbossche]

@jreback @toobaz ready for review

I went through the tests and added back some of the tests removed in #18882

I also added back MultiIndex._reference_duplicate_name to use it in stack/unstack (so we can there raise an informative error message). However, I would personally prefer that MultiIndex._get_level_number directly raises this informative warning, then the extra checking in stack/unstack can be removed. However, that is a slight change in behaviour for eg get_level_values (KeyError -> ValueError), so I would rather leave that for 0.24.0.

[jorisvandenbossche]

However, I would personally prefer that MultiIndex._get_level_number directly raises this informative warning

That was also the original intent of:

pandas/pandas/core/indexes/multi.py

Lines 771 to 776 in a620e72

	count = self.names.count(level)
	if count > 1:
	raise ValueError('The name %s occurs multiple times, use a '
	'level number' % level)
	level = self.names.index(level)
	except ValueError:

where already an informative ValueError is raised, but now the ValueError is catched, and then a generic KeyError is raised (this was changed 5 years ago: bdf270c#diff-dc96e62847cdfe77b570cf9c6b0e8086R1536)

[TomAugspurger]

Until we don't exactly know what those future plans are, I would rather keep it limited (only noting that duplicated level names are allowed again?)

Agreed.

[jreback]

I would rather not add API here. why don't you just do this check inside _get_level_number itself, and then also add to Index so you don't also have to add an instance check?

[jorisvandenbossche]

Did you read my comment in the main thread?
This code above is exactly how it was before. As I said, I also think this is better in get_level_number, but propose to do a follow-up PR that can be merged for 0.24.0, as it gives an additional API change (change in error type)

[jreback]

then let's just wait till 0.24.0 to do this.

[TomAugspurger]

It seems like a good fix for the regression, why not merge it? It's no different that reverting a previous commit.

[jorisvandenbossche]

yes, it is how it was for a long time, and this is fixing a regression so it is the point to do it now and not wait for 0.24.0.

I can certainly do the PR for 0.24.0 to further clean this up, but that will involve an API change so I will not do that in this PR.

[jreback]

fair enough, is there an issue to clean this up?

[jorisvandenbossche]

@jreback I am doing a PR, but ran into some issues: #21677

[jorisvandenbossche]

Opened the PR to not forget his (#21678), but as mentioned, we first need to discuss the other issue.