TYP: rename
State: closed

tested with mypy and pyright:

```py
import pandas as pd

def bool_() -> bool:
    ...

df = pd.DataFrame({"a": []})
ser = pd.Series({"a": 1})

# DataFrame/Series
reveal_type(df.rename(columns={"a": "b"}))
reveal_type(df.rename(columns={"a": "b"}, inplace=False))
reveal_type(ser.rename("a"))
reveal_type(ser.rename("a", inplace=False))

# None
reveal_type(df.rename(columns={"a": "b"}, inplace=True))
reveal_type(ser.rename("a", inplace=True))

# both
reveal_type(df.rename(columns={"a": "b"}, inplace=bool_()))
reveal_type(ser.rename("a", inplace=bool_()))
```

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

Comments:
Hello @twoertwein! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:











There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2022-04-01 12:11:24 UTC
Marking as draft as #46423 should be merged first. Then this PR can add a few more `IgnoreRaise` to `rename`.
Thank you @Dr-Irv for reviewing my PRs :) If you think I can provide feedback for your PRs here or at MS, feel free to ping me.

I slowly try to get the errors from pyright's `reportGeneralTypeIssues` to manageable amount to enable it at some point (#44855). Many (easier) to fix errors are caused by missing overloads (mypy assumes Any but pyright inspects all possible return values which then creates error).
> I think I found a nicer solution that works with the latest pyright+mypy and is sanctions by [one of the typing gods](https://github.com/microsoft/pyright/issues/2344#issuecomment-927341908):

hmm, this is really the issue here, that Mapping is invariant in its key type, xref https://github.com/python/mypy/issues/12464#issuecomment-1079752807

the response was to use Any as the key type as a workaround. While this is probably fine for stubs since a dict/mapping can only be created at runtime with hashable keys, this workaround does not help with the type checking of the function itself, as we would not know the type of the key inside the function. (Using `Any` also prevents us turning on some of the strict flags down the line.)

I fear that due to other questions in that thread that we didn't get a resolution on this one.

So while the solution here using the TypeVars seems to satisfy the requirements of both users checking their own code and checking the internals of pandas itself, it still feels a bit kludgy. 

@twoertwein in you discussions with mypy/pyright regarding this did you come across other projects having this issue? Is there a precendent for using TypeVars as a workaround for Mapping being invariant in its key type?
could we maybe use Any in the overloads (the public API) and Hashable in the signature of function itself (internal code checking)?
> So while the solution here using the TypeVars seems to satisfy the requirements of both users checking their own code and checking the internals of pandas itself, it still feels a bit kludgy.

I agree.  IMHO, if we want to use a type for the keys of `Dict` or `Mapping`, it can only be a base type (e.g., `str`, `int`) or we use `Any`.  We can't use a `Union` (e.g., `Dict[int|str, str]` won't work).  I don't think we can use `Hashable`, because essentially `Hashable` is a `Union` (although declared as a `Protocol`).

> or we use `Any`

since that was "official" response from the mypy side I think we should probably do that for now.

This does not satisfy internal code checking requirements, hence the suggestion above https://github.com/pandas-dev/pandas/pull/46428#issuecomment-1084726875
I think I finally agree with `Mapping[Any, Hashable]` in case of rename as pandas will only ever call it with Hasables and it is guaranteed to get a Hashable in return. (If we had code that inverts keys and values, then that wouldn't be okay with me.)



> @twoertwein in you discussions with mypy/pyright regarding this did you come across other projects having this issue? Is there a precendent for using TypeVars as a workaround for Mapping being invariant in its key type?

No other projects but two of my "issues" at pyright https://github.com/microsoft/pyright/issues/2344#issuecomment-927341908 (pyright: explicit use case to get covariant behavior) and https://github.com/microsoft/pyright/issues/3272 (no objections by pyright and mypy maintainers for implementing covariant keys with typevars - but also no approval)


The only issue I have with `Mapping[Any, Hashable]` is that the keys of Mapping are not required to be Hashable (and neither are the arguments to a Callable):

```py
from typing import Any, Hashable, Mapping, TypeVar

HashableT = TypeVar("HashableT", bound=Hashable)


def get_mapper() -> Mapping[list[int], int]:
    ...


def any_rename(mapper: Mapping[Any, Hashable]) -> None:
    ...


def typevar_rename(mapper: Mapping[HashableT, Hashable]) -> None:
    ...


# allowed - Mapping does not require the keys to be Hashable (Dict does)
any_rename(get_mapper())

# error (both mypy and pyright)
typevar_rename(get_mapper())
``` 

But I fully agree that the TypeVar solution is quite verbose when we have functions where we need multiple TypeVars to express the same type constraint (on the otherhand it will also work with any non-generic type including `int | str`).




> So while in the past we tried to be as precise as possible with the type annotations, i'm fine with less precise types in these situations (for now!)

I'll go with `Any`

> Using `Any` also prevents us turning on some of the strict flags down the line.

I think we are unfortunately quite far from that point. I would prioritize py.typed over having no `Any`s (typeshed itself also has a few `Any`s).
My takeaways for the future:

- Avoid TypeVar magic when it is getting too verbose
- When the first argument of Mapping/Dict/List/Callable is Hashable and the type is used as an input argument, replacing Hashable with Any seems like a good solution
- For types stricter than Hashable, TypeVar magic is a solution (maybe even the only solution) but it might get verbose
> I pulled your latest version, and ran the following test

Thank you for testing! When I run mypy and pyright (both on python 3.10), I do not get a single error (except for unused variables from pyright).
> > I pulled your latest version, and ran the following test
> 
> Thank you for testing! When I run mypy and pyright (both on python 3.10), I do not get a single error (except for unused variables from pyright).

My mistake.  I had a typo when I pulled the PR.  Test passes for me now.
thanks @twoertwein