CLN: Remove MultiIndex._get_grouper_for_level
State: closed

- [x] closes #49452 (Replace xxxx with the GitHub issue number)
- [ ] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [x] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [x] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) to new arguments/methods/functions.
- [ ] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.


Labels:
name: MultiIndex
name: Clean

Comments:
I think according to https://github.com/pandas-dev/pandas/issues/49452#issue-1432037433, we would like this to return a `NotImplementedError`
> I think according to [#49452 (comment)](https://github.com/pandas-dev/pandas/issues/49452#issue-1432037433), we would like this to return a `NotImplementedError`

I think so - the idea being that for a MultiIndex, the current implementation of `Index._get_grouper_for_level` would likely give odd errors or incorrect results if called on a MultiIndex. We should fail clearly if this is attempted.
Ok no problem.  I'll push again today.

Sorry for some reason I thought I'd read a conversation where the decision was to remove it entirely instead of raise.
if its just a 1-liner, id suggest putting it in the base class method with a quick check

```
if self._is_multi:
    raise NotImplementedError(...)
```

that way you can add `@final` to the Index method, which I always find helpful
> if its just a 1-liner, id suggest putting it in the base class method with a quick check
> 
> ```
> if self._is_multi:
>     raise NotImplementedError(...)
> ```
> 
> that way you can add `@final` to the Index method, which I always find helpful

Okay implemented as suggested in this last commit.  But a few more comments/questions:

It seems to me that `level` is now only being asserted in `_get_grouper_for_level` but not used for anything else (and `dropna` not used at all).  It looks like either `grouper` is set to the index or whatever mapping the mapper does on it and the only argument being used in here is `mapper`.  If it's `@final` too then is there a reason to keep those extra arguments in?  I'm sure there's a good reason and I'm just not familiar enough with any of the code base.
> If it's `@final` too then is there a reason to keep those extra arguments in? I'm sure there's a good reason and I'm just not familiar enough with any of the code base.

Prior to https://github.com/pandas-dev/pandas/pull/49373 they were used for MultiIndex, but now they no longer are. These can be removed - and doing so here would be great.
Looks like something went awry with pulling from main. In general, you should only need to do `git pull upstream main` in order to incorporate changes from main into your branch. At this point, there are a few options, but maybe the easiest would be to just siphon off your changes into a new branch. You can see changes from individual commits or consecutive groups of commits using the UI on github in the "Files changed" tab.
> Looks like something went awry with pulling from main. In general, you should only need to do `git pull upstream main` in order to incorporate changes from main into your branch. At this point, there are a few options, but maybe the easiest would be to just siphon off your changes into a new branch. You can see changes from individual commits or consecutive groups of commits using the UI on github in the "Files changed" tab.

Ya my bad, I rebased my branch.  I'm going to just start a new branch as suggested.

While removing the args from the Index class it looks like _get_grouper_for_level is returning extra Nones as well.  In a new clean branch I'll make all the changes I think can happen to clean up and you can tell me if I've gone too far?  Unfortunately still haven't been able to run the tests locally.  I'm using the docker container and panda-dev environment.  Maybe a fresh pull etc and this will all be cleared up but to be sure, I should expect the tests to all pass locally right?

Thank you!
> I rebased my branch.

When I rebase, I first make sure all my changes are on origin (in case something goes wrong) and then do:

```
git merge-base HEAD upstream/main
[prints a commit hash]
git rebase -i [commit hash] -X theirs
```

Then pick the first commit and squash the rest. Sometimes if there are interleaving changes you get conflicts that need to be manually resolved, but most of the time everything goes through cleanly.

> I'll make all the changes I think can happen to clean up and you can tell me if I've gone too far? 

Sounds good!

> I should expect the tests to all pass locally right?

For the most part, yes. Sometimes there are flakey tests, but otherwise, if the CI is green and your dev environment was created based on what's currently in main then the tests should pass.
Will open a new one shortly