TYP fixup overloads for reset_index

[MarcoGorelli]

• Ensure all linting tests pass, see here for how to run them

Took me longer than I'm proud to admit, but I think I've finally understood the deal with overload and default types.

We need one overload for the default case (here, inplace=False), and then an extra overload of each combination of keyword arguments which precede inplace.

So here, we need:

• one case for the defaults
• one case for when there all all arguments and inplace=True
• one case for when there's no level, and inplace=True
• one case for when there's no drop, and inplace=True
• one case for when there's no level, no drop, and inplace=True

Then, mypy will be able to correctly infer the return type for reset_index based on the value of inplace.

This kind of thing will help resolve the mypy issues seen in #38594, where the return type from fillna couldn't be determined. It also means not needing to add the ignore error from #37137

---

Some examples:

# t.py

import pandas as pd

reveal_type(pd.DataFrame([1,2,3]).reset_index(level=0))
reveal_type(pd.DataFrame([1,2,3]).reset_index(inplace=False))
reveal_type(pd.DataFrame([1,2,3]).reset_index(drop=True, inplace=True))
reveal_type(pd.DataFrame([1,2,3]).reset_index(level=0, inplace=True))
reveal_type(pd.DataFrame([1,2,3]).reset_index(level=0, drop=False, inplace=True))
reveal_type(pd.DataFrame([1,2,3]).reset_index(level=0, inplace=True, col_fill='a'))
reveal_type(pd.DataFrame([1,2,3]).reset_index(level=0, col_fill='a'))
reveal_type(pd.DataFrame([1,2,3]).reset_index(inplace=True))

$ mypy t.py 
t.py:3: note: Revealed type is 'pandas.core.frame.DataFrame'
t.py:4: note: Revealed type is 'pandas.core.frame.DataFrame'
t.py:5: note: Revealed type is 'None'
t.py:6: note: Revealed type is 'None'
t.py:7: note: Revealed type is 'None'
t.py:8: note: Revealed type is 'None'
t.py:9: note: Revealed type is 'pandas.core.frame.DataFrame'
t.py:10: note: Revealed type is 'None'

[simonjayhawkins]

MarcoGorelli requested a review from simonjayhawkins 15 minutes ago

watch this space!

[simonjayhawkins]

MarcoGorelli requested a review from simonjayhawkins 15 minutes ago

watch this space!

see #40202

[simonjayhawkins]

removing the type annotations from the function means that we lose the type checking for the body of the function.

so if you reveal_type on say level you will get Revealed type is 'Any'

IIUC correctly, the type annotations on the function are used to check the function itself, while the overloads are used when checking other functions that call this one.

so both are needed.

[simonjayhawkins]

Is the last overload needed now? won't it fall back to the function definition if no overloads match?

updated: comment should have been on the next one.

[MarcoGorelli]

Pretty sure it's needed, see here for a toy example (uncomment the commented-out part and it'll pass)

[MarcoGorelli]

Commenting out the last overload leads to the same errors observed on master in the typing test from #40202 (comment)

[simonjayhawkins]

ah ok, and Revealed type is 'Any'. I thought that it fell back to the non-overloaded function with no matching overloads. maybe something changed in mypy or only does so under different conditions.

[MarcoGorelli]

only does so under different conditions

If I've understood correctly, this is it - see #39501 (comment)

[simonjayhawkins]

Thanks @MarcoGorelli it would be great to have some testing in place before merging this, not because there could be anything wrong with the additions, but because susequent changes could easily break things. but we are where we are, so merge when ready.

[MarcoGorelli]

Thanks - OK, merging then as I'm not sure how long it'll take for #40202 to be ready, have opened #40349 to make sure that adding tests for this isn't forgotten

[simonjayhawkins]

have opened #40349 to make sure that adding tests for this isn't forgotten

great. we can add the reveals from your comments here to the tests in #40202

[simonjayhawkins]

just one more thing!

these extra overloads are needed because of an issue with mypy. we have now lost that link to the issue.

although the mypy issue was initially dismissed, it appears that there is acknowledgement that the workaround can becomes impractical with many arguments. There therefore may be a fix on the mypy side in the future. We somehow need to track that issue.

[MarcoGorelli]

The (*, level, inplace) overload should probably have been (level, *, inplace), but I'll check tomorrow