Skip to content

TYP overload fillna #40737 #40887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Apr 15, 2021
Merged

TYP overload fillna #40737 #40887

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
79f6b50
TYP: Added overloads for fillna() in frame.py and series.py
Apr 11, 2021
9b6966a
TYP: Added overloads for fillna() in frame.py and series.py #40737
Apr 11, 2021
fce2d11
TYP: Added fillna() overloads to generic.py #40727
Apr 11, 2021
bd93f31
Merge branch 'typ-fillna' of github.com:LarWong/pandas into typ-fillna
Apr 11, 2021
16b42e7
TYP: removed generic overloads #40737
Apr 11, 2021
2ab41f8
fixed redundant cast error
Apr 12, 2021
d6cc081
Merge remote-tracking branch 'upstream/master' into typ-fillna
Apr 12, 2021
a641df2
reverting prior changes
Apr 12, 2021
45cc60c
remove cast again
Apr 12, 2021
1c33bd5
Merge remote-tracking branch 'upstream/master' into typ-fillna
Apr 12, 2021
b6a4fac
removed unnecessary overloads in frame.py and series.py
Apr 12, 2021
2adcaf7
fixed overloads
Apr 12, 2021
4b7744e
Merge remote-tracking branch 'upstream/master' into typ-fillna
Apr 15, 2021
0c3d28d
reverted value typing
Apr 15, 2021
1c8dd1a
remove extra types (lets keep this to overloads)
MarcoGorelli Apr 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
348 changes: 348 additions & 0 deletions pandas/core/frame.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5007,6 +5007,354 @@ def rename(
errors=errors,
)

@overload
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we not put these in .pyi?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was told to add them directly to these files since existing overloads were already there

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jreback yes, that would be great! Thing is though, the .pyi files require you to define all methods of a module

Given the sheer number of methods this module has, I'd suggest taking this PR with the overloads here, and then moving the overloads (along with annotations for all other methods) to a pandas/core/frame.pyi file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LarWong I'll get back to you on typing value

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we not put these in .pyi?

I don't think we want to go the route of using stubs for python files

@jreback yes, that would be great! Thing is though, the .pyi files require you to define all methods of a module

not that I think it should be done here, but it is possible to partially type a module using stubs.

https://github.com/python/typeshed/blob/master/CONTRIBUTING.md#incomplete-stubs

Partial modules (i.e. modules that are missing some or all classes, functions, or attributes) must include a top-level getattr() function marked with an # incomplete comment (see example below).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @simonjayhawkins , didn't know that was possible. Why not use partial stubs for overloads? Because some methods, like Series.drop

pandas/pandas/core/series.py

Lines 4478 to 4487 in 84d9c5e

def drop(
self,
labels=None,
axis=0,
index=None,
columns=None,
level=None,
inplace=False,
errors="raise",
) -> Series:

have 5 (five!) arguments with defaults before inplace, leading to...34 overloads 🤯 ! And even if we disallowed labels being passed as None in that one, that would still leave us with 18 overloads!

For typing value here, do you think Scalar | Mapping[Hashable, Scalar] | Series | DataFrame | None would be correct?

Copy link
Member

@simonjayhawkins simonjayhawkins Apr 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that if a stub file is present it takes precedence over the python file. so we cannot ensure internal consistency and need to duplicate the type annotations to be able to check the functions in the module itself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have 5 (five!) arguments with defaults before inplace, leading to...34 overloads

I think we are planning to to drop the inplace argument, hopefully pandas 2.0 and we won't need all these overloads. #16529

def fillna(
self,
value: Scalar | dict | Series | DataFrame | None = ...,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make an alias for this

method: str | None = ...,
axis: Axis | None = ...,
inplace: Literal[False] = ...,
limit: int | None = ...,
downcast: dict | str | None = ...,
) -> DataFrame:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
method: str | None,
axis: Axis | None,
inplace: Literal[True],
limit: int | None,
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
*,
inplace: Literal[True],
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
*,
inplace: Literal[True],
) -> None:
...

@overload
def fillna(
self,
*,
method: str | None,
inplace: Literal[True],
) -> None:
...

@overload
def fillna(
self,
*,
axis: Axis | None,
inplace: Literal[True],
) -> None:
...

@overload
def fillna(
self,
*,
inplace: Literal[True],
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
method: str | None,
*,
inplace: Literal[True],
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
*,
axis: Axis | None,
inplace: Literal[True],
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
*,
inplace: Literal[True],
limit: int | None,
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
*,
inplace: Literal[True],
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
*,
method: str | None,
axis: Axis | None,
inplace: Literal[True],
) -> None:
...

@overload
def fillna(
self,
*,
method: str | None,
inplace: Literal[True],
limit: int | None,
) -> None:
...

@overload
def fillna(
self,
*,
method: str | None,
inplace: Literal[True],
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
*,
axis: Axis | None,
inplace: Literal[True],
limit: int | None,
) -> None:
...

@overload
def fillna(
self,
*,
axis: Axis | None,
inplace: Literal[True],
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
*,
inplace: Literal[True],
limit: int | None,
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
method: str | None,
axis: Axis | None,
inplace: Literal[True],
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
method: str | None,
*,
inplace: Literal[True],
limit: int | None,
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
method: str | None,
*,
inplace: Literal[True],
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
*,
axis: Axis | None,
inplace: Literal[True],
limit: int | None,
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
*,
axis: Axis | None,
inplace: Literal[True],
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
*,
inplace: Literal[True],
limit: int | None,
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
*,
method: str | None,
axis: Axis | None,
inplace: Literal[True],
limit: int | None,
) -> None:
...

@overload
def fillna(
self,
*,
method: str | None,
axis: Axis | None,
inplace: Literal[True],
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
*,
method: str | None,
inplace: Literal[True],
limit: int | None,
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
*,
axis: Axis | None,
inplace: Literal[True],
limit: int | None,
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
*,
method: str | None,
axis: Axis | None,
inplace: Literal[True],
limit: int | None,
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
*,
axis: Axis | None,
inplace: Literal[True],
limit: int | None,
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
method: str | None,
*,
inplace: Literal[True],
limit: int | None,
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
method: str | None,
axis: Axis | None,
inplace: Literal[True],
*,
downcast: dict | str | None,
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None,
method: str | None,
axis: Axis | None,
inplace: Literal[True],
limit: int | None,
) -> None:
...

@overload
def fillna(
self,
value: Scalar | dict | Series | DataFrame | None = ...,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you've typed value in the overloads, perhaps let's type it in the function signature too? (line 5128)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can modify it, but let's wait until someone has confirmed the typing of value first.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LarWong perhaps let's revert typing value here, that can be done separately (and we'll probably need to be more precise than dict), the rest looks good

Copy link
Contributor Author

@LarWong LarWong Apr 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MarcoGorelli Sorry, but I'm not sure what you mean by revert typing. Do you mean:

    @overload
    def fillna(
        self,
        value,

As in not typing value in the overloads?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MarcoGorelli Ok done! The outputs from mypy are the same after modifications (see above).

method: str | None = ...,
axis: Axis | None = ...,
inplace: bool = ...,
limit: int | None = ...,
downcast: dict | str | None = ...,
) -> DataFrame | None:
...

@doc(NDFrame.fillna, **_shared_doc_kwargs)
def fillna(
self,
Expand Down
Loading