add some type annotations io/formats/format.py #27418

simonjayhawkins · 2019-07-16T15:43:54Z

typing not added to all function signatures in io/formats/format.py in this pass to reduce the diff

this PR also contains a refactor to simplify the addition of type annotations; truncate_h and tr_col_num are dependant variables. truncate_h can be True or False. if True, tr_col_num is an int else if truncate_h is False, tr_col_num is None

~~same applies for truncate_v and tr_row_num~~

jbrockmendel · 2019-07-16T16:35:43Z

pandas/io/formats/format.py

        fmt_values = self._get_formatted_values()

-        result = ["{i}".format(i=i) for i in fmt_values]
+        result = ["{i}".format(i=i) for i in fmt_values]  # type: Union[str, List[str]]


would it be cleaner if this were just List[str] and then on 187 use a different variable name?

refactored.

WillAyd · 2019-07-16T17:56:32Z

pandas/io/formats/format.py

+    def __init__(
+        self,
+        categorical: ABCCategorical,
+        buf: Optional[StringIO] = None,


Minor but I think should use typing.TextIO here instead

probably, but it's not yet failing mypy, so doing it 'gradually'. will change on next commit.

WillAyd · 2019-07-16T17:58:33Z

pandas/io/formats/format.py

+        dtype: bool = True,
+        max_rows: Optional[int] = None,
+        min_rows: Optional[int] = None,
+    ) -> None:


Can remove this for init - pseudo-standard internally not to add (though was required by earlier mypy versions)

WillAyd · 2019-07-16T17:59:18Z

pandas/io/formats/format.py

                series = concat((series.iloc[:row_num], series.iloc[-row_num:]))
-            self.tr_row_num = row_num
-        else:
-            self.tr_row_num = None


Just not required anymore?

no. see OP.

WillAyd · 2019-07-16T18:00:45Z

pandas/io/formats/format.py

        else:
            self.columns = frame.columns

+        self.truncate_h = None  # type: Optional[int]


If this gets assigned to an int subsequently and None isn't valid I think should just type as int.

There's some syntax trickery at play here due to py 3.5 support as in py36 forward you could declare this without initializing I think like:

self.truncate_h: int self.truncate_v: int

But can't do that in 3.5

None is valid, so that we get the same behaviour as False when it was bool

None is when the DataFrame is not truncated. the assigned int is the truncated row/column number.

WillAyd · 2019-07-16T18:02:00Z

pandas/io/formats/format.py

        frame = self.frame
-        if truncate_h:
-            if max_cols_adj == 0:
-                col_num = len(frame.columns)


Is this not a valid path?

truncate_h is defined on L548. if max_cols_adj==0, truncate_h is False?

…ping-and-refactor

simonjayhawkins · 2019-07-16T20:22:28Z

@WillAyd to reduce the diff and confusion, i've added tr_row_num (and tr_col_num) back in, but defined truncate_v ( and truncate_h) using tr_row_num (and tr_col_num) to appease the static type checking

…ping-and-refactor

WillAyd · 2019-07-19T17:09:11Z

pandas/io/formats/format.py


-    def _chk_truncate(self):
+    @property
+    def truncate_v(self) -> bool:


Have to excuse my lack of knowledge with this module but this isn't entirely consistent with the previous logic right? Any risk in implementing this differently?

removed properties

WillAyd · 2019-07-19T17:10:46Z

pandas/io/formats/format.py

    is_timedelta64_dtype,
 )
 from pandas.core.dtypes.generic import (
+    ABCCategorical,


Not a blocker but generally have been moving away from using the ABC classes in annotations as they don't actually represent anything statically and just get replaced with Any.

Does importing the actual object impact things negatively here?

removed ABCs

jreback · 2019-07-20T20:03:16Z

lgtm. @WillAyd

WillAyd

Looks very good. Minor stylistic nits and a question but otherwise lgtm

WillAyd · 2019-07-20T21:52:19Z

pandas/io/formats/format.py

        truncate_v = max_rows and (len(self.series) > max_rows)
        series = self.series
        if truncate_v:
+            max_rows = cast(int, max_rows)


Is this required? Generally we try to avoid cast and I'm surprised MyPy can't narrow the type down here from None.

If it's fully required does making line 238 say max_rows is not None and ... get around that?

If it's fully required does making line 238 say max_rows is not None and ... get around that?

no. gives pandas\io\formats\format.py:249: error: Unsupported operand types for // ("None" and "int") and there might be risk in implementing this differently anyway?

Generally we try to avoid cast

well I would have agreed with you when I started this, but after this PR, I think that adding type annotations should be just that and any refactoring or code cleanup should be deferred to a follow on PR.

I'm surprised MyPy can't narrow the type down here from None.

Agreed. hence the numerous iterations to try and avoid cast originally.

Hmm OK yea seems like a bug / gap with mypy inference then. Can always come back to clean up

WillAyd · 2019-07-20T21:53:35Z

pandas/io/formats/html.py

        return 0

-    def _get_columns_formatted_values(self) -> ABCIndex:
+    def _get_columns_formatted_values(self) -> Iterable[Any]:


Can just make this Iterable - if you don't subscript the generics they default to inclusive of Any anyway

WillAyd · 2019-07-20T21:55:57Z

pandas/io/formats/html.py

    def write_tr(
        self,
-        line: List[str],
+        line: Iterable[Any],


Same thing here for Iterable

WillAyd · 2019-07-21T01:06:54Z

Thanks @simonjayhawkins !

simonjayhawkins · 2019-07-21T09:09:45Z

thanks @WillAyd for the review.

add some type annotations and refactor io/formats/format.py

c0291fa

simonjayhawkins added Refactor Internal refactoring of code Output-Formatting __repr__ of pandas objects, to_string Clean Typing type annotations, mypy/pyright type checking labels Jul 16, 2019

blackify

5df8ffa

jbrockmendel reviewed Jul 16, 2019

View reviewed changes

simonjayhawkins added 2 commits July 16, 2019 18:46

refactor to remove redefinition of variable with incompatible type

b73d522

simplify

ce71739

WillAyd requested changes Jul 16, 2019

View reviewed changes

simonjayhawkins added 5 commits July 16, 2019 19:27

refactor _get_column_name_list

32d3b29

remove NoneType return value from __init__

3cff805

use typing.TextIO instead of StringIO

2c37a0b

Merge remote-tracking branch 'upstream/master' into format-partial-ty…

a8cf8e1

…ping-and-refactor

reduce diff

db4bfe7

simonjayhawkins added 2 commits July 16, 2019 21:25

blackify

0ac539b

Merge remote-tracking branch 'upstream/master' into format-partial-ty…

ff47366

…ping-and-refactor

WillAyd reviewed Jul 19, 2019

View reviewed changes

simonjayhawkins added 3 commits July 19, 2019 19:41

remove properties

480c808

remove abcs

7dc2af0

revert refactor of _chk_truncate

035ec58

simonjayhawkins changed the title ~~add some type annotations and refactor io/formats/format.py~~ add some type annotations io/formats/format.py Jul 20, 2019

simonjayhawkins removed Clean Refactor Internal refactoring of code labels Jul 20, 2019

simonjayhawkins added 2 commits July 20, 2019 20:12

revert other refactorings

a58f1cd

add type hint to _get_columns_formatted_values was abc

66f6069

jreback added this to the 1.0 milestone Jul 20, 2019

WillAyd requested changes Jul 20, 2019

View reviewed changes

remove Any from Iterable

dd3c46d

WillAyd approved these changes Jul 21, 2019

View reviewed changes

WillAyd merged commit 88ccb25 into pandas-dev:master Jul 21, 2019

simonjayhawkins mentioned this pull request Jul 22, 2019

more type hints for io/formats/format.py #27512

Merged

simonjayhawkins deleted the format-partial-typing-and-refactor branch July 22, 2019 11:48

quintusdias pushed a commit to quintusdias/pandas_dev that referenced this pull request Aug 16, 2019

add some type annotations io/formats/format.py (pandas-dev#27418)

8c971d9

simonjayhawkins mentioned this pull request Sep 10, 2019

Annotate DataFrame (Part 1) #26867

Merged

4 tasks

simonjayhawkins mentioned this pull request Mar 12, 2022

TYP: returning None for __init__ #46337

Closed

Uh oh!

add some type annotations io/formats/format.py #27418

add some type annotations io/formats/format.py #27418

Uh oh!

Conversation

simonjayhawkins commented Jul 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonjayhawkins commented Jul 16, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback commented Jul 20, 2019

Uh oh!

WillAyd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WillAyd commented Jul 21, 2019

Uh oh!

simonjayhawkins commented Jul 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

simonjayhawkins commented Jul 16, 2019 •

edited

Loading