TYP: try out TypeGuard
State: closed

This effectively makes mypy understand that `is_integer(obj)` is equivalent to `isinstance(obj, (int, np.integer))`.   This is a mixed bag.  It allows for getting rid of some redundant casts/ignores, but makes some new ones necessary.  I'd be fine either way on this.

Labels:
name: Typing
description: type annotations, mypy/pyright type checking

Comments:
One place in util._validators where making pyright and mypy both pass at the same time is a hassle

  /home/runner/work/pandas/pandas/pandas/util/_validators.py:262:12 - error: Expression of type "bool | bool_ | int | BoolishNoneT@validate_bool_kwarg | None" cannot be assigned to return type "BoolishNoneT@validate_bool_kwarg"
    Type "bool | bool_ | int | BoolishNoneT@validate_bool_kwarg | None" cannot be assigned to type "BoolishNoneT@validate_bool_kwarg" (reportGeneralTypeIssues)

Tried annotating `Index._is_multi` as `TypeGuard[MultiIndex]` hoping a bunch of existing ignores would become redundant, no luck
I've made a new version, could you take look?

I dislike union typeguard unless a clear reason, because they require casts or checks in other locations,  but I've  kept them anyway in this version. @jorisvandenbossche , what's your opinion on those, e.g. `def is_integer(val: object) -> TypeGuard[int | np.integer]`?
> Not looked in detail but from my attempts at using TypeGuard, agree that Union types can cause problems. (This was a while back and not up-to-date on best practice on this)

Looking at the places in this PR where `cast` had to be done to handle cases where you wanted `int` and not `np.integer`, I think it would be better if we had more explicit `is_*` functions.  So in `_libs/lib.pyi`, rather than 
```python
def is_bool(val: object) -> TypeGuard[bool | np.bool_]: ...
def is_integer(val: object) -> TypeGuard[int | np.integer]: ...
```
you would have:
```python
def is_bool(val: object) -> TypeGuard[bool | np.bool_]: ...
def is_integer(val: object) -> TypeGuard[int | np.integer]: ...
def is_pybool(val: object) -> TypeGuard[bool]: ...
def is_pyinteger(val: object) -> TypeGuard[int]: ...
```
Then in `pandas/core/generic.py`, around line 4076, instead of
```python
            loc, new_index = index._get_loc_level(key, level=0)
            if not drop_level:
                if lib.is_integer(loc):
                    loc = cast(int, loc)
                    new_index = index[loc : loc + 1]
```
you could remove the cast by doing
```python
            loc, new_index = index._get_loc_level(key, level=0)
            if not drop_level:
                if lib.is_pyinteger(loc):
                    new_index = index[loc : loc + 1]
```


> For evolving/defining more precise types, pandas-stubs is now our playground so cc @Dr-Irv for comment

I would alter that statement a bit by saying that `pandas-stubs` is meant to provide types for the public API, as opposed to being precise.  For something like the `is_*` functions, they are for internal use, so if `TypeGuard` helps, use it.

As for a dependency on `typing_extensions`, I think this had to be done when we still supported python 3.7 and were using `Literal` in typing constructs.  So if we were willing to use `typing_extensions` back then, we should be OK to use it now.


> As for a dependency on `typing_extensions`, I think this had to be done when we still supported python 3.7 and were using `Literal` in typing constructs. So if we were willing to use `typing_extensions` back then, we should be OK to use it now.

IIRC we never used `typing_extensions` and have only ever used workarounds. 
> > As for a dependency on `typing_extensions`, I think this had to be done when we still supported python 3.7 and were using `Literal` in typing constructs. So if we were willing to use `typing_extensions` back then, we should be OK to use it now.
> 
> IIRC we never used `typing_extensions` and have only ever used workarounds.

Yes, I agree. I was mixing up some of our own application code with stuff in pandas.  It seems that in pandas, we always surrounded an import of `Literal` with an `if TYPE_CHECKING`, and the CI would do type checking using python 3.8, even though we still supported python 3.7.

Which does suggest that maybe the solution to avoid using `typing_extensions` is to just copy the code for `TypeGuard` from `typing_extensions`, i.e.:
```python
if sys.version_info >= (3, 10):
    from typing import TypeGuard
else:  # Copied from typing_extensions
    import typing

    class _TypeGuardForm(typing._SpecialForm, _root=True):
        def __repr__(self):
            return "typing_extensions." + self._name

    @_TypeGuardForm
    def TypeGuard(self, parameters):
        item = typing._type_check(parameters, f"{self} accepts only a single type.")
        return typing._GenericAlias(self, (item,))
```

I don't know if this will work, and you might need to add some `#type: ignore` and/or `#pyright: ignore` statements in there, but just copying what is in `typing_extensions` ought to work.

It might also be worth considering whether it is worth using python 3.10 or higher in CI for type checking, in which case you don't need the whole `else` section above, because any annotations with `TypeGuard` would be ignored at runtime.

> @topper-123 just noticed a type:ignore where we do `is_list_like(obj) and len(obj) == ...` where mypy doesn't get that obj is listlike.  could we address that with a TypeGuard on is_list_like?

From a short glance I don't think that is possible: The TypeGuard would be (in addition to list, ndarray) *any iterable, except strings and bytes*.  Exceptions like that can't be represented in the type system to my knowledge.

The closest we have in the type system is `_typing.ListLike`, which is `ExtensionArray | np.ndarray | Index | Series | List | range`.
I think this PR looks pretty good but want to go back to not using cast that @simonjayhawkins and I touched on before. Can those be removed? Replaced with ignore at least? 

Didn't see a response on that - sorry if overlooking 
> I think this PR looks pretty good but want to go back to not using cast that @simonjayhawkins and I touched on before. Can those be removed? Replaced with ignore at least?

I can make a updated version that uses ignores rather than casts. I'll ping when I've done it.

@Dr-Irv , a problem with implementing `is_pyinteger` etc. IMO is that if a `np.integer` subtype is supplied to those locations it will not be accepted. E.g.

```python
if is_pyinteger(obj):
   # do the right thing for ints
else:
   # do the wrong things for ints
```
We'll risk that `np.integer` types will go to the `else` clause, which will increase the risk of errors, AFAIKS.

Also if we wanted is_py_integer wouldn't we just use `isinstance(obj, int)`?
Ive made a version without casts. The errors seem unrelated.
@simonjayhawkins you seem to have a blocking request. Could you see if everything is ok?  
Thanks @topper-123

The block was only to give a chance for feedback on the typing_extensions discussion.

I'm a bit skeptical that TypeGuard is a silver bullet for all is_* functions.

For instance the dtype functions can return True for a Dtype object or an array having that dtype.

```
def is_bool_dtype(arr_or_dtype) -> bool:
    """
    Check whether the provided array or dtype is of a boolean dtype.

    Parameters
    ----------
    arr_or_dtype : array-like or dtype
        The array or dtype to check.
```

I think if we introduce a union return for these, we will require too many ignores/casts/asserts.

Similarly, I think it is unlikely to help with functions such as is_array_like unless perhaps we use Protocols instead on union return types.

For the uses in this PR, we have union return types to include the the numpy types which aren't subtypes of the Python types. I recall looking at this previously (around the time that we introduced the numpy types) and I think that the general rule is that where we accept a Python type we also probably accept a numpy type, but not necessarily the other way. So, in theory, we should not really need any ignores/casts/asserts in most cases and could define aliases in _typing for the unions of the Python and Numpy types and use those throughout.

Side note: I see a use of asserts in this PR. It is subjective and we use assert with None in many places, but in general this always introduces the potential to raise an AssertionError at run-time.

otherwise lgtm 
Great work @jbrockmendel @topper-123 
> Similarly, I think it is unlikely to help with functions such as is_array_like unless perhaps we use Protocols instead on union return types.

Interesting conversation. I've fallen behind on the latest typing functionality but maybe some combination of overloads on the argument type + TypeGuard as the return type could solve this issue too?