Revert set_index inspection/error handling for 0.24.1
State: closed

Reverts #24762 and #22486 as an alternative to #24984 for 0.24.1. I've kept the docstring updates from #24762, because they are independent of the code changes (otherwise the docstring validation would throw up anyway).

There were conflicts for `test_set_index_pass_single_array` and `test_set_index_pass_arrays_duplicate`, where I've tried to err on the side of inclusiveness. The rest of reversions in `/tests/` I didn't touch - those remain in master anyway.

Labels:
name: Docs
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Error Reporting
description: Incorrect or improved errors from pandas

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/25085?src=pr&el=h1) Report
> Merging [#25085](https://codecov.io/gh/pandas-dev/pandas/pull/25085?src=pr&el=desc) into [0.24.x](https://codecov.io/gh/pandas-dev/pandas/commit/5cb622a8a6bf36726ccaa6f39aa63302af533fd9?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           0.24.x   #25085      +/-   ##
==========================================
+ Coverage   92.36%   92.37%   +<.01%     
==========================================
  Files         166      166              
  Lines       52398    52391       -7     
==========================================
- Hits        48400    48394       -6     
+ Misses       3998     3997       -1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.79% <100%> (-0.01%)` | :arrow_down: |
| #single | `42.87% <62.5%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/25085?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/25085/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `96.81% <100%> (-0.02%)` | :arrow_down: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/25085/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `88.13% <0%> (+0.09%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/25085?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/25085?src=pr&el=footer). Last update [5cb622a...999295e](https://codecov.io/gh/pandas-dev/pandas/pull/25085?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/25085?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@0d3b4f4`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff            @@
##             master   #25085   +/-   ##
=========================================
  Coverage          ?   92.37%           
=========================================
  Files             ?      166           
  Lines             ?    52408           
  Branches          ?        0           
=========================================
  Hits              ?    48411           
  Misses            ?     3997           
  Partials          ?        0
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.79% <100%> (?)` | |
| #single | `42.86% <75%> (?)` | |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/25085?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/25085/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `96.81% <100%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/25085?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/25085?src=pr&el=footer). Last update [0d3b4f4...65c7880](https://codecov.io/gh/pandas-dev/pandas/pull/25085?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

cc @jreback @WillAyd 

Pushed an empty commit, since azure didn't test the original one.
Anyone have throughs  here vs. 
https://github.com/pandas-dev/pandas/pull/24984?

@jorisvandenbossche?
I personally don't think the full revert is necessarily (as I said on the other PR), so *I* would go forward with the other PR. But if this PR is the way to get everybody on board on the short term, that's certainly fine with me.
Do you prefer reverting to doing nothing for 0.24.1?

> On Feb 2, 2019, at 10:26, Joris Van den Bossche <notifications@github.com> wrote:
> 
> I personally don't think the full revert is necessarily (as I said on the other PR), so I would go forward with the other PR. But if this PR is the way to get everybody on board on the short term, that's certainly fine with me.
> 
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub, or mute the thread.

Slight preference to reverting then
Let's merge this then?
> @jreback [above]: could leave these tests and just xfail them.

> @jreback [[in 24984](https://github.com/pandas-dev/pandas/pull/24984#issuecomment-459714413)]: I think it would be better to completely revert (though leave the tests)

Done both of that with the last commit.
> @jorisvandenbossche: Let's merge this then?

Just saw your comment now - feel free to revert the last two (non-merge) commits if that is preferable.
@h-vetinari I changed the target to master for this branch (for the bug fix releases, for most PRs we merge to master and only after that backport to 0.24.x)
> @jorisvandenbossche: I changed the target to master for this branch (for the bug fix releases, for most PRs we merge to master and only after that backport to 0.24.x)

I know, but I purposefully made it to 0.24.x, because master will be solved by #24984, and then the diff there will get more complicated. I still think that #24984 would be the better solution for 0.24.1 anyway, but OK; I I've already given you free hand in changing this PR and the other, so please do whatever you think is necessary.
> I know, but I purposefully made it to 0.24.x, because master will be solved by #24984, and then the diff there will get more complicated.

But you are adding many of the tests of the other PR here as well, so the diff will get *smaller* there, no?

> I still think that #24984 would be the better solution for 0.24.1 anyway, but OK;

I agree, but that is the compromise we made.
> But you are adding many of the tests of the other PR here as well, so the diff will get smaller there, no?

Smaller for tests, bigger for `frame.py`

> I agree, but that is the compromise we made.

Which I am trying to support with this PR.
> Smaller for tests, bigger for frame.py

OK, but then at least it is clear what we are changing again, compared to the revert in 0.24.1 (in any case, merging to master is the standard workflow, any deviation should be have a good reason / should be discussed, and IMO it will be cleaner to merge to master).

> Which I am trying to support with this PR.

For which we are very greatful!

> Smaller for tests, bigger for frame.py

@h-vetinari BTW, can I already advise you to split the bug fix on the error when setting an index twice and dropping it into a separate PR? That a clear, fine clean-up that should not need much discussion otherwise.
> @jorisvandenbossche: BTW, can I already advise you to split the bug fix on the error when setting an index twice and dropping it into a separate PR? That a clear, fine clean-up that should not need much discussion otherwise.

It's a two-line code change and very easy to understand. I'm thinking it could be included in this PR already? This would also avoid the need to change anything about `test_set_index_pass_arrays_duplicate`.
>  I'm thinking it could be included in this PR already? 

I fully agree, but let's just get this merged first. Tom wants to tag more or less now, to be able to release this evening. So let's run CI, and merge this.
> I fully agree, but let's just get this merged first. Tom wants to tag more or less now, to be able to release this evening. So let's run CI, and merge this.

I actually meant for the release. If someone already uses `.set_index(['A', 'A'], drop=True)`, we'd be regressing again... Again, just throwing ideas out there - you continue however you want to play it.
Yeah, go ahead. That also keeps the note in 0.24.0 half correct.
Merging on green.
Thanks @h-vetinari!