BUG: NumericIndex should not support float16 dtype
State: closed

- [x] closes #49535
- [x] [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).
- [] 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.

The whatsnew does not need updating, because `NumericIndex` is purely internal.

Extracted from #49494

Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Index
description: Related to the Index class or subclasses

Comments:
Will `Index(..., dtype=np.float16)` upcast to `np.float32` in the future then too?

Makes sense from an indexing utility why float16 should be disallowed, but just as a container it could be?

```
In [7]: pd.Index([1], dtype=np.float16)
Out[7]: Float64Index([1.0], dtype='float64')  # Would this be Index(..., dtype=float32) in the future?

In [8]: pd.Series([1], dtype=np.float16)
Out[8]:
0    1.0
dtype: float16
```

cc @jbrockmendel if you have any opinions on consistency with the above example
> Will Index(..., dtype=np.float16) upcast to np.float32 in the future then too?

I would much rather this raise than give you something unexpected.

More generally, it'd be nice if Index supported everything Series did
My reasoning for upcasting was that pandas has a history of being very permissive with inputs and doing a lot to take in everything. This is not a very strong opinion of mine, so I'm also ok with disallowing `float16`.

`float16` is not available in khash, which was needed to make an index in the same style as the other numeric indexes, but yeah I would also preferred to not special case `float16`.
@topper-123 would it be possible upcast to float32 when needing to dispatching to khash but keep the float16 when storing values (this may be a naive question with my gaps in indexing code).

Agreed with Brock that generally we should be more explicit with disallowing float16 in Index (and I guess Series) or supporting float16 in Index & Series (maybe upcasting internally to float32 in Index when needed?)
Are we sure there won't be lossiness if we use float32 in the backend for float16 indexes?

I'm thinking that e.g. unions of float16 and int8 indexes then need to convert the int8->float32 float16->float32 and then convert the result back down to float16. Could that give wrong results in some circumstances? I'm also not a super fan of the special casing needed in the code and tests to make that happen, because float16 is rarely used.

Can we instead use @jbrockmendel suggestion to disallow float16 indexes? If someone later wants to have float16 index they casn open a PR at that time?

> unions of float16 and int8 indexes then need to convert 

I'm assuming that `find_common_dtype` (or the associated dtype resolution path) would just upcast to the appropriate type, but agreed that I may be underestimating edge cases where just-in-time promotions may cause issues.

I'd be okay raising a `NotImplementedError` for now to disallow float16
disallowing float16 would be OK.  my preference would be to implement Float16Engine in index.pyx to cast to float32 before passing things to the hashtables.
I've updated to raise `NotImplementedError` in float16 cases.
```
cdef class Float16Engine:
    def __init__(self, values):
        values = values.astype(np.float32)
        super().__init__(values)

    def get_loc(self, key):
        if isinstance(key, np.float16):
            key = key.astype(np.float32)
        return super().get_loc(key)

    def get_indexer(self, other):
        other = other.astype(np.float32)
        return super().get_indexer(other)
```
Yeah could be. I got another way using 16-bit floats in 32-bit IndexEngines in #49560 file pandas/core/indexes/numeric.py, (lines 103 and 116), so that's not a blocker ATM.

We could discuss which is better, though I do like the simplicity of mine :-).
Yeah I think 

```
    def _get_engine_target(self) -> ArrayLike:
        vals = self._values
        # pandas has no Float16Engine, so we use Float32Engine instead
        if vals.dtype == "float16":
            vals = vals.astype("float32")
        return vals
```

in https://github.com/pandas-dev/pandas/pull/49560 looks reasonable (given the test passing) 
I've looked into making `Float16Engine` as suggested by @jbrockmendel in order to support float16 indexes. It may not be practically possible (without a lot of work). The underlying issue is that `cnumpy` doesn't have a `float16_t`. Without a `float16_t` type, we can't give float16 arrays to the relevant c-level functions and will have make workarounds, which will probably be ugly and of marginal value (no one uses float16 arrays anyway).

For further discussion see [this](https://stackoverflow.com/questions/47421443/using-half-precision-numpy-floats-in-cython) and [this](https://stackoverflow.com/questions/38975770/python-numpy-float16-datatype-operations-and-float8) stackoverflow discussions. From the discussions it looks like that numpy internally converts float16 to float32 and then after the operations converts back to float16. I guess we could do that also, but that may be a project unto itself and outside of scope my current work (i.e. collecting numeric indexes in the base `Index`).

I'm thinking we should choose between:
1: make instantiating `Index` with a float16 dtype raise a `NotImplementedError`.
2: make instantiating `Index` with a float16 dtype convert to `Index` with a `float32` dtype (maybe raise a warning also?).

It seems from the discussion above that the majority opinion is 1. Do you still hold that opinion given that numpy does not have a `float16_t` type?

i guess go forward with this and ill take a stab at implementing Float16Engine
umm we don't support float16 for nearly anything. 

would be -1 on supporting 
just raise
Something to consider is that calling any numpy ufunc on int8 arrays returns a float16 array e.g.

```python
>>> import numpy as np
>>> arr = np.arange(3, dtype=npint8)
>>> np.exp(arr)
array([1.   , 2.719, 7.39 ], dtype=float16)
```

If we raise on float16 indexes, calling those ufunc functions on int8 indexes would also raise unless we guard against that in `Index__array_ufunc__`, e.g.

```python
>>> import pandas as pd
>>> idx = pd.core.api.NumericIndex(arr)
>>> idx
NumericIndex([0, 1, 2], dtype='int8')
>>> np.exp(idx)  # what should this return?
```

The choices are (if we do not have a float16 index):
1. raise NotImplentedError
2. return `NumericIndex(np.exp(arr), dtype='float32')` by changing changing float 16 arrays to float32 arrays in `Index.__array_ufunc__`
3. return `NumericIndex(np.exp(arr), dtype='float32')` by  letting `NumericIndex(arr, dtype='float16')` return a float32 index (not raising on float16 instantiation)?

Raising on a `NumericIndex(..., dtype=float16)` input means either choice 1 or 2. Option 1 would be the most stringent but would fail in some cases where users might expect it to not fail, while option 2 is a bit like option 3 but without allowing directly instantiating with `dtype=float16` become float32 indexes.

Any comment, especially on the issue of ufuncs on int8 indexes?
> Any comment, especially on the issue of ufuncs on int8 indexes?

We could cast float16 ufunc results to float32 before wrapping in an Index.  I think we do something similar with FloatingArray.

I still think best-case is to support the same dtypes in Index that we do with Series.  I've got a branch going that implements Float16Engine, have some test failures to work out.
> We could cast float16 ufunc results to float32 before wrapping in an Index. I think we do something similar with FloatingArray.

That is what I've done in the newest version (option 2 above). If we could pull this in (and #50195) then we'd  be ready to pull in #49560 also and I could proceed with removing Int64Index etc?

I just rebased, just in case, and the failure looks unrelated.

Could we merge this and then the work that @jbrockmendel does in #50218 could be rebased to include this PR? This one not being merged is blocking #49560, which is an important step in the work of making the base `Index` take numeric dtypes. 
The failures look unrelated to this PR.
My intention was to write the whatsnew after getting the the code PRs committed, because it's all related. Would that be ok (I'll include the float16 issue in).
Sure 
👍 Can we pull this PR in (after the CI gets working again)?  
I’ve rebased so it would pass the CI. No other changes were made.
I'd like to merge this, is that ok?
Thanks @topper-123