TYP: pandas/core/frame.py #38416

arw2019 · 2020-12-11T19:20:59Z

I plan to do a pass through the whole file - putting it up in case anybody has feedback on what I have

pandas/core/frame.py

jreback · 2020-12-11T23:26:01Z

suggest that you do reasonably sized pieces so this doesn't sit in review for too long - better multiple but smaller prs as this is a huge file

jreback

would prefer you don't mix easy and hard ones. e.g. a pass on bool / strings ones. and Axis / Level. These are easy to review. by mixing lots of things its very hard to give good scrutiny here.

would prefer that.

jreback · 2020-12-12T23:10:38Z

pandas/core/frame.py

+        data,
+        orient: str = "columns",
+        dtype: Optional[Dtype] = None,
+        columns: Optional[List] = None,


i believe this should be Arraylike here

do you mean columns? The doctring says list (columns is actually column labels so it seems to make sense)

I think it should be Optional[List[Label]]

jreback · 2020-12-12T23:11:23Z

pandas/core/frame.py

@@ -1440,7 +1456,7 @@ def to_numpy(

        return result

-    def to_dict(self, orient="dict", into=dict):
+    def to_dict(self, orient: str = "dict", into=dict) -> Union[Dict, List, Mapping]:


Mapping should be enough here, why did you add List?

with orient="records" we return a list:

pandas/pandas/core/frame.py

Lines 1589 to 1599 in 36c4d5c

elif orient == "records":

columns = self.columns.tolist()

rows = (

dict(zip(columns, row))

for row in self.itertuples(index=False, name=None)

)

return [

into_c((k, maybe_box_datetimelike(v)) for k, v in row.items())

for row in rows

]

You can at least get rid of Dict then - no reason to include Dict and Mapping unless I am overlooking something

Yes, that's right - done

with orient="records" we return a list:

we should probably overload using Literal to avoid Union return types. but not in this pass.

jreback · 2020-12-12T23:11:51Z

pandas/core/frame.py

@@ -1161,7 +1168,7 @@ def __len__(self) -> int:
        """
        return len(self.index)

-    def dot(self, other):
+    def dot(self, other: Union[AnyArrayLike, DataFrame]) -> FrameOrSeriesUnion:


be explict and add Series to these

pandas/core/frame.py

jreback · 2020-12-12T23:13:25Z

pandas/core/frame.py

@@ -3311,7 +3329,7 @@ def _box_col_values(self, values, loc: int) -> Series:
    # ----------------------------------------------------------------------
    # Unsorted

-    def query(self, expr, inplace=False, **kwargs):
+    def query(self, expr: str, inplace: bool = False, **kwargs) -> Optional[DataFrame]:


i don't think this is correct for the output type

I think this is right. From docstring:

DataFrame resulting from the provided query expression or None if ``inplace=True``.

no my point is that the doc-string is wrong. this can be a Series or Scalar. basically this can be anything. I would remove it for now.

Ok - removed (and will fix the docstring in follow-on, too)

jreback · 2020-12-12T23:13:34Z

pandas/core/frame.py

-    def eval(self, expr, inplace=False, **kwargs):
+    def eval(
+        self, expr: str, inplace: bool = False, **kwargs
+    ) -> Optional[Union[AnyArrayLike, Scalar]]:


the output type here is suspect

From docstring:

Returns ------- ndarray, scalar, pandas object, or None The result of the evaluation or None if ``inplace=True``.

so does Optional[Union[AnyArrayLike, DataFrame, Scalar]] work?

pandas/core/frame.py

jreback · 2020-12-12T23:16:01Z

pandas/core/frame.py

-    def apply(self, func, axis=0, raw=False, result_type=None, args=(), **kwds):
+    def apply(
+        self,
+        func: Callable,


I believe we are more specific on this, though it maybe difficult

Looking - maybe we could do something like what was done for AggFuncType

jreback · 2020-12-12T23:17:35Z

pandas/core/reshape/pivot.py

@@ -612,7 +612,7 @@ def crosstab(
        margins=margins,
        margins_name=margins_name,
        dropna=dropna,
-        **kwargs,
+        **kwargs,  # type: ignore[arg-type]


don't add type ignores

okay I'll skip pivot in this PR (this is a weird issue with typing **kwargs)

jreback · 2020-12-12T23:19:08Z

cc @simonjayhawkins @WillAyd

arw2019 · 2020-12-13T04:35:23Z

would prefer you don't mix easy and hard ones. e.g. a pass on bool / strings ones. and Axis / Level. These are easy to review. by mixing lots of things its very hard to give good scrutiny here.

would prefer that.

For sure. I'll keep the hard ones that I've already attempted here so all the comments are in one place. I'll resubmit easy and follow-ons in separate PRs

jreback · 2020-12-14T14:25:26Z

needs merge master and will look

arw2019 · 2020-12-14T19:56:39Z

mypy green + all comments addressed (or moved out to orthogonal PRs)

WillAyd

Looks pretty good - just a few things

WillAyd · 2020-12-14T23:04:29Z

pandas/core/frame.py

@@ -1440,7 +1456,7 @@ def to_numpy(

        return result

-    def to_dict(self, orient="dict", into=dict):
+    def to_dict(self, orient: str = "dict", into=dict) -> Union[Dict, List, Mapping]:


You can at least get rid of Dict then - no reason to include Dict and Mapping unless I am overlooking something

pandas/core/frame.py

WillAyd · 2020-12-14T23:05:49Z

pandas/core/frame.py

@@ -7802,7 +7828,7 @@ def apply(
        )
        return op.get_result()

-    def applymap(self, func, na_action: Optional[str] = None) -> DataFrame:
+    def applymap(self, func: Callable, na_action: Optional[str] = None) -> DataFrame:


Can this be typed more strictly than Callable? Subscripting Callable is much more useful to a reader if possible

Yes - @jreback asked for this too. I'll put up a follow-on for this (to avoid expanding the scope of this PR)

…ping-frame

github-actions · 2021-02-06T00:12:29Z

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

jreback · 2021-02-11T01:28:30Z

@arw2019 if you want to merge master and update (or close and open a new one ok too)

arw2019 · 2021-02-22T00:08:39Z

@jreback @simonjayhawkins updated, CI green, comments addressed

pandas/core/frame.py

…ping-frame

simonjayhawkins · 2021-03-14T13:07:41Z

@jreback @simonjayhawkins updated, CI green, comments addressed

ok. maybe we should merge this. there has been plenty of discussion here.

simonjayhawkins · 2021-03-15T13:28:21Z

@jreback @WillAyd i'm merging later today if no objections.

mypy is green and typing PRs easily become stale or get abandoned.

There are no ignores, casts or asserts added here and no existing annotations changed/deleted.

if any of the typing additions added here are problematic they will be picked up by mypy as more types are added.

jreback · 2021-03-15T13:39:46Z

sure let's make sure this is rebased first

simonjayhawkins · 2021-03-15T14:00:57Z

yep did that locally and ran mypy before I commented. will push and merge on green

jreback

lgtm

simonjayhawkins · 2021-03-15T15:32:40Z

Thanks @arw2019

arw2019 added 4 commits December 11, 2020 14:18

typing

46bf318

typing

eba7251

typing

1a45267

typing

f5cb185

jbrockmendel reviewed Dec 11, 2020

View reviewed changes

pandas/core/frame.py Outdated Show resolved Hide resolved

jbrockmendel reviewed Dec 11, 2020

View reviewed changes

pandas/core/frame.py Outdated Show resolved Hide resolved

review comments

4c9b764

arw2019 changed the title ~~[WIP] TYP: pandas/core/frame.py~~ TYP: pandas/core/frame.py Dec 11, 2020

jreback requested changes Dec 12, 2020

View reviewed changes

jreback added the Typing type annotations, mypy/pyright type checking label Dec 12, 2020

This was referenced Dec 13, 2020

TYP: pandas/core/frame.py (easy: bool/str) #38440

Merged

TYP: pandas/core/frame.py (easy: Axis/Level) #38441

Merged

arw2019 added 3 commits December 13, 2020 00:53

review comment

a273e5c

review comment

e0558c5

merge master

e156caa

arw2019 mentioned this pull request Dec 14, 2020

TYP: DataFrame.to_gbq, DataFrame.to_html (easy: copy-paste from format module) #38461

Merged

5 tasks

arw2019 added 6 commits December 14, 2020 12:52

merge master

13ff245

revert hints to pivot_table

617e228

take out merge

9a4d187

minimize diff

77e1142

fix eval return type annotation

2352e45

merge master

a3b7b1b

WillAyd requested changes Dec 14, 2020

View reviewed changes

arw2019 added 2 commits December 14, 2020 20:43

review comments

1c942fd

Merge branch 'master' of https://github.com/pandas-dev/pandas into ty…

0e747dd

…ping-frame

github-actions bot added the Stale label Feb 6, 2021

merge master

a28deaf

arw2019 mentioned this pull request Feb 19, 2021

TYP: create a Frequency alias in _typing #39919

Merged

4 tasks

arw2019 added 2 commits February 21, 2021 13:07

merge master

9243c6b

typing

58fcad8