BUG: Index.get_loc raising incorrect error, closes #29189
State: closed

- [x] closes #29189
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

cc @WillAyd 

Labels:
name: Groupby
name: Error Reporting
description: Incorrect or improved errors from pandas

Comments:
> Hmm what do you think about just catching the TypeError in groupby? Seems a little strange to catch and re-raise as a KeyError

Personally I like restricting the types of errors raised from our indexing codee, but based on other examples in pandas, I am also not fully sure the TypeError is "wrong" or the KeyError is "correct" (we are not being very consistent ..). 
For example, here are three different errors for three different "mistyped" values (values that cannot be in the index based on their type):

```python
>>> s = pd.Series(range(5), index=[1, 2, 3, 4, 4])  
```
```python
>>> s['a']   
...
~/miniconda3/lib/python3.7/site-packages/pandas/core/indexes/base.py in get_value(self, series, key)
   4721         k = self._convert_scalar_indexer(k, kind="getitem")
   4722         try:
-> 4723             return self._engine.get_value(s, k, tz=getattr(series.dtype, "tz", None))
   4724         except KeyError as e1:
   4725             if len(self) > 0 and (self.holds_integer() or self.is_boolean()):
...
pandas/_libs/index.pyx in pandas._libs.index.IndexEngine._get_loc_duplicates()

KeyError: 'a'
```
```python
>>> s[2.5]
...
   4719         k = com.values_from_object(key)
   4720 
-> 4721         k = self._convert_scalar_indexer(k, kind="getitem")
   4722         try:
   4723             return self._engine.get_value(s, k, tz=getattr(series.dtype, "tz", None))
...
~/miniconda3/lib/python3.7/site-packages/pandas/core/indexes/base.py in _convert_scalar_indexer(self, key, kind)
   3111             if kind in ["getitem", "ix"] and is_float(key):
   3112                 if not self.is_floating():
-> 3113                     return self._invalid_indexer("label", key)
   3114 
   3115             elif kind in ["loc"] and is_float(key):
...
TypeError: cannot do label indexing on <class 'pandas.core.indexes.numeric.Int64Index'> with these indexers [2.5] of <class 'float'>
```
```python
>>> s[None]
...
pandas/_libs/index.pyx in pandas._libs.index.IndexEngine._get_loc_duplicates()

TypeError: '<' not supported between instances of 'NoneType' and 'NoneType'

During handling of the above exception, another exception occurred:

IndexError                                Traceback (most recent call last)
<ipython-input-22-32b37d43effc> in <module>
----> 1 s[None]

~/miniconda3/lib/python3.7/site-packages/pandas/core/series.py in __getitem__(self, key)
   1062         key = com.apply_if_callable(key, self)
   1063         try:
-> 1064             result = self.index.get_value(self, key)
   1065 
   1066             if not is_scalar(result):

~/miniconda3/lib/python3.7/site-packages/pandas/core/indexes/base.py in get_value(self, series, key)
   4741             # python 3
   4742             if is_scalar(key):  # pragma: no cover
-> 4743                 raise IndexError(key)
   4744             raise InvalidIndexError(key)
   4745 

IndexError: None
```

For non-duplicated index or for RangeIndex, the case of `s[None]` also gives KeyError, so in that regard (consistency with how `None` is handled in other index types), the KeyError seems to be a better option.
@WillAyd @jreback @jorisvandenbossche thanks for weighing in.

The overall "what should indexing methods raise" issue is a pretty big one and I'd like to segment the problem if possible.  Can we achieve consensus on any of the following:

1) get_loc _should_ raise `TypeError` if passed a non-hashable
2) The exception raised by get_loc should _not_ depend on whether the index is unique
3) The exception raised by get_loc should _not_ depend on whether the index is monotonic

Anything else that can be added to this list?
> 1. get_loc _should_ raise `TypeError` if passed a non-hashable

Hmm not sure about hashability making a difference. I was thinking more if we try to index say a `IntIndex` with a string that it should raise a TypeError since the `IntIndex` would never hold a string. But I think Jeff is saying that this should return -1. Not an expert on usage here so I'll defer

But yes to points 2 and 3 - I wouldn't think either of those should have an impact on the Exception that gets raised
@WillAyd for the purposes of 1. I'm not looking to say anything _shouldn't_ raise `TypeError`, just that non-hashables _should_ (and currently do; its the first thing checked by each of the get_loc implementations in index.pyx)
> @WillAyd @jreback @jorisvandenbossche thanks for weighing in.
> 
> The overall "what should indexing methods raise" issue is a pretty big one and I'd like to segment the problem if possible. Can we achieve consensus on any of the following:
> 
> 1. get_loc _should_ raise `TypeError` if passed a non-hashable
> 2. The exception raised by get_loc should _not_ depend on whether the index is unique
> 3. The exception raised by get_loc should _not_ depend on whether the index is monotonic
> 
> Anything else that can be added to this list?

2 and 3 for certain yes.

1 yes as well ideally. likely though we *don't* do this in many places and are translating a TypeError to -1 and returning. This would likely involved catching the TypeError at a higherlevel (and then raising KeyError which is ok).
rebase & can you add a whatsnew note here for the bug fix
@WillAyd any objection to moving forward with this as a bugfix and revisiting the larger get_loc raising behavior separately?
No objections from me
thanks