DEPR: remove ix
State: closed

- [x] Needs release note
- [ ] Needs geopandas compat
- [x] Need to remove ix asvs

Labels:
name: Deprecate
description: Functionality to remove in pandas

Comments:
@jbrockmendel lgtm. can you PR to geopandas to fix?
can you add the issue number at the top
closing, will revisit once ix deprecation is enforced
What do you mean exactly? I think we want to remove it now?
You're right, it was the `NDFrameIndexer.__getitem__` that we're keeping around a little while longer until geopandas stops relying on it.  Either way, this is low priority for me at the moment.
> Either way, this is low priority for me at the moment.

OK, but let's keep the PR open for now. Then it is clearer that there is already a PR to start from in case somebody wants to do the work (and I think we still want this done for 1.0)
@jbrockmendel can you merge master and address comments?
Closing to clear the queue.  Will reopen when this becomes a priority.
Sounds good. FWIW I think this is close so happy to commit review resource as needed when you have time
IMO this is a priority for 1.0
Woops, meant to reopen, not delete.  Will have to re-push in a new PR I think
Can't you push to the same name? I thought it was only a problem to reopen a PR after a force push (although apparently you can circumvent that :) https://gist.github.com/robertpainsi/2c42c15f1ce6dab03a0675348edd4e2c)
Re-closing.  AFAICT new tests have been added that rely on ix, not interested in re-writing those ATM.
It's bizarre that those tests didn't fail earlier, as the ones that are failing now are multiple years old (so certainly not recent tests).

They are also annoying to rewrite, they are all based on `check_result` and basically doing something like this:

```
        self.check_result(
            "int label", "loc", 2, "ix", 2, typs=["ints", "uints"], axes=0
        )
```

Where the `loc` or `iloc` result is compared with `ix` result. 
But since we are removing `ix`, we don't have that anymore to compare with. In principle we could compare with iloc with loc and loc with iloc? But eg since all the objects are different, to compare iloc with loc, you need to know the exact label for each of the type of objects (before `ix` would fall back to positional, so you didn't need this)

We coud also remove those tests? (or al least xfail then for now, so we can merge the useful parts of this PR?)

@jbrockmendel if you'd merge master and i'll have a final look. we should merge soon.
@jreback we fist need to decide what to do with the tests discussed in https://github.com/pandas-dev/pandas/pull/27620#issuecomment-536168251 
Not that I now just commented out a bunch of tests to get this passing


> Check with Joris to see who’s working on the ix deprecation

To clarify this, I don't have the intent (or at least not the time the coming two weeks) to do much work on it. With my pushed changes I mainly wanted to understand the issue why Brock was getting so annoyed with it to close it (because I find it important to get this done with). And see https://github.com/pandas-dev/pandas/pull/27620#issuecomment-536168251 for the summary, it are indeed rather horrible tests to work with and to rewrite. 

I personally find it important enough to get this merged that I would maybe just skip those tests for now (the build is only green because I commented them out), and leave that for a follow-up. 

There are some other review comments from before that still might need attention, that I didn't check.


i guess should fix the testing mechanism first, would propose that we just can in the test specifications hardcode the list of indexers that we should get an use iloc as the benchmark (of course that doesn't work for iloc testing itself), but at least can check the indexers.

I think we should fix the testing but am not too hung up about this as currently this is *really* just testing that we match for the indexers, rather than testing the ground truth, so in some sense these tests give us a false sense of security.
@jbrockmendel do you have time to update this?
just rebased, but that may have undone whatever commits not-me pushed to this branch
This needs a bit more work than just rebasing, so my question was rather about that

(and indeed, you undid the changes I pushed. Pushed it to a separate branch, the commit is here: https://github.com/jorisvandenbossche/pandas/commit/0be1470d106e0927f2606f0488f1f972b240c684)
As I've said a few times, I am not especially interested in this.  If you'd like to take it over, go ahead.
rebased
@jorisvandenbossche is the geopandas compat problem resolved?  if so, does this just need a whatsnew note?
@jorisvandenbossche gentle ping, we're running out of deprecations to enforce
lgtm generally, 2 items which i think should be done in this PR
updated+green
I am ok with merging this, long overdue.

I would say create an issue to make sure that for .loc we have enough tests that absolute expected cases (rather than .ix comparisons).

@pandas-dev/pandas-core any objections.
Yes, let's handle further testing clean-up in a follow-up issue.

> @jorisvandenbossche is the geopandas compat problem resolved?

It's still in a released version (the same as when we discussed this a few months ago), but it is already fixed in the meantime. I would prefer to keep the compat code here a little bit longer (the geopandas version that fixes it is only recently released)

@jbrockmendel thanks a *lot* for this!