TYPING: some type hints for pandas\io\common.py #27598

simonjayhawkins · 2019-07-25T23:37:11Z

No description provided.

pandas/io/common.py

WillAyd · 2019-07-26T01:17:38Z

pandas/io/common.py

-    def __init__(self, file, mode, compression=zipfile.ZIP_DEFLATED, **kwargs):
+    def __init__(
+        self,
+        file: Union[BinaryIO, str],


Hmm did FilePathOrBuffer not work here? Does ZipFile not accept pathlib.Path objects?

it works. will change.

WillAyd · 2019-07-26T01:23:42Z

pandas/io/common.py

@@ -511,5 +526,7 @@ def UnicodeReader(f, dialect=csv.excel, encoding="utf-8", **kwds):
    return csv.reader(f, dialect=dialect, **kwds)


-def UnicodeWriter(f, dialect=csv.excel, encoding="utf-8", **kwds):
+def UnicodeWriter(
+    f: TextIO, dialect: Type[csv.excel] = csv.excel, encoding: str = "utf-8", **kwds


Suggested change

f: TextIO, dialect: Type[csv.excel] = csv.excel, encoding: str = "utf-8", **kwds

f: TextIO, dialect: Type[csv.Dialect] = csv.excel, encoding: str = "utf-8", **kwds

WillAyd · 2019-07-26T01:24:36Z

pandas/io/formats/format.py

@@ -730,6 +731,11 @@ def to_string(self) -> None:
        """
        Render a DataFrame to a console-friendly tabular output.
        """
+        # Note: the to_string method only accepts IO whereas to_html and


Thanks for the notes - are these bugs?

These are the sort of inconsistencies between to_html, to_string and to_latex that I was hoping to find by adding type hints.

This should help with refactoring the three methods to share buffer handling code.

Can't really call them bugs when the documentation doesn't say that filenames are accepted (even though they are for to_html and to_latex).

buf : StringIO-like, optional Buffer to write to.

Do you know what the lifecycle of self.buf = cast(...) is? Would that cast still just stay local to the function?

WillAyd · 2019-07-26T04:19:31Z

pandas/io/common.py

@@ -144,21 +147,22 @@ def _stringify_path(filepath_or_buffer):
    strings, buffers, or anything else that's not even path-like.
    """
    if hasattr(filepath_or_buffer, "__fspath__"):
-        return filepath_or_buffer.__fspath__()
+        # https://github.com/python/mypy/issues/1424


When we drop 3.5 support do you think we can just do `isinstance(filepath_or_buffer, os.PathLike) here instead?

WillAyd · 2019-07-26T04:22:18Z

pandas/io/formats/format.py

@@ -730,6 +731,11 @@ def to_string(self) -> None:
        """
        Render a DataFrame to a console-friendly tabular output.
        """
+        # Note: the to_string method only accepts IO whereas to_html and


Do you know what the lifecycle of self.buf = cast(...) is? Would that cast still just stay local to the function?

simonjayhawkins · 2019-07-28T10:20:33Z

On hindsight, i shouldn't have used cast... i was just avoiding adding a ignore.

I think in this case, since a TypeError is raised at runtime, an ignore should have been used for the static check.

…d test

simonjayhawkins · 2019-07-30T12:57:57Z

some changes so that one of the errors is now
pandas\io\formats\format.py:911: error: Argument 1 to "buffer_put_lines" has incompatible type "Union[str, Path, IO[str]]"; expected "IO[str]"

instead of
pandas\io\formats\format.py:905: error: Argument 1 to "buffer_put_lines" has incompatible type "Union[str, Path, IO[Any]]"; expected "TextIO"

still needs a # type: ignore anyway because of the usage of hasattr

jreback · 2019-07-31T12:07:29Z

pandas/io/formats/format.py

+        if hasattr(buf, "write"):
+            yield buf
+        elif isinstance(buf, str):
+            import codecs


import at the top

jreback · 2019-08-02T13:23:08Z

pandas/io/formats/format.py

+
+    def get_result(
+        self,
+        buf: Optional[FilePathOrBuffer[str]] = None,


can you followup with some doc-strings for these

jreback · 2019-08-02T13:24:09Z

small comments, @WillAyd

WillAyd

Nice change / refactor one minor question

WillAyd · 2019-08-02T14:43:48Z

pandas/io/common.py

@@ -93,7 +96,9 @@ def _is_url(url):
        return False


-def _expand_user(filepath_or_buffer):
+def _expand_user(
+    filepath_or_buffer: FilePathOrBuffer[AnyStr]


What does subscripting with AnyStr do here? Add to Union?

FilePathOrBuffer is a just a Union. FilePathOrBuffer[AnyStr] with subscription is effectively a TypeVar in IO. so IO[str] can't become IO[bytes].

i think https://mypy.readthedocs.io/en/latest/generics.html#generic-type-aliases explains it. will send a different link if i come across a better explanation.

That's pretty cool - thanks for sharing! Does this really change anything though? We already have IO[AnyStr] in FilePathOrBuffer so this just restates that (?)

Unrelated note - IO[AnyStr] might itself be wrong as AnyStr is a TypeVar and I think we need to parametrize IO with the actual type. I find the Python docs rather confusing on that so maybe we remove AnyStr altogether but can level that as a separate exercise (unless it helps simplify annotation here)

Unrelated note - IO[AnyStr] might itself be wrong as AnyStr is a TypeVar and I think we need to parametrize IO with the actual type.

The alias is a Union, the Union has one and only one Generic. so parametrising the alias is parametrising the only Generic in the Union, i.e. IO.

We already have IO[AnyStr] in FilePathOrBuffer so this just restates that (?)

I don't think AnyStr is treated as a TypeVar inside the union when the alias is defined. so that's why it's needed in use

Does this really change anything though?

yes. mypy will fail without it.

in to_html etc only string buffers are supported. hence Optional[FilePathOrBuffer[str]] is used (note the parametrisation of FilePathOrBuffer here) and the TypeVar then becomes necessary otherwise a bytes buffer could be returned.

buffer_put_lines(buf: IO[str], lines: List[str]) -> None only supports string buffers. so mypy will raise if we don't use TypeVars to maintain the FilePathOrBuffer type in and out of the common functions.

Right I think the std documentation isn't very clear but just defining IO creates a generic accepting a type of AnyStr which is str / bytes. This is in contrast to other generics that really accept type T (essentially anything). You can see this if you try to inject a non-str or bytes type

from typing import IO foo: IO[int]

yields

error: Value of type variable "AnyStr" of "IO" cannot be "int"

So I think an error to keep re-parametrizing IO with AnyStr in the _typing module and here.

Let's leave to a follow up to clean up though

WillAyd · 2019-08-02T17:14:28Z

Thanks @simonjayhawkins

TYPING: some type hints for pandas\io\common.py

9421bb9

simonjayhawkins added Output-Formatting __repr__ of pandas objects, to_string Typing type annotations, mypy/pyright type checking labels Jul 25, 2019

simonjayhawkins added 2 commits July 26, 2019 00:41

BytesIO -> BinaryIO

0e4a1d9

Merge remote-tracking branch 'upstream/master' into pandas-io-common.py

3abd219

WillAyd requested changes Jul 26, 2019

View reviewed changes

address comments (WillAyd)

0b96604

WillAyd reviewed Jul 26, 2019

View reviewed changes

simonjayhawkins closed this Jul 28, 2019

simonjayhawkins added 5 commits July 30, 2019 09:41

Merge remote-tracking branch 'upstream/master' into pandas-io-common.py

749237e

remove ignores and casts

dac39b3

revert addition of return value to get_filepath_or_buffer

32caa9f

fix AttributeError: 'str' object has no attribute 'writelines' and ad…

7502435

…d test

use FilePathOrBuffer as TypeVar

ab3f546

simonjayhawkins reopened this Jul 30, 2019

simonjayhawkins added 2 commits July 30, 2019 22:40

refactor filepath_or_buffer handling

07980b9

add test for bad arg

9886585

jreback reviewed Jul 31, 2019

View reviewed changes

simonjayhawkins added 4 commits July 31, 2019 14:19

move import to top

0d6af90

Merge remote-tracking branch 'upstream/master' into pandas-io-common.py

2701be9

resolve merge conflicts

3c71335

Merge remote-tracking branch 'upstream/master' into pandas-io-common.py

171b03a

jreback added this to the 1.0 milestone Aug 2, 2019

jreback approved these changes Aug 2, 2019

View reviewed changes

WillAyd reviewed Aug 2, 2019

View reviewed changes

WillAyd approved these changes Aug 2, 2019

View reviewed changes

WillAyd merged commit 15b11c8 into pandas-dev:master Aug 2, 2019

WillAyd mentioned this pull request Aug 2, 2019

Removed continued parametrization of FilePathOrBuffer #27719

Closed

simonjayhawkins mentioned this pull request Aug 4, 2019

DOC: update docstrings following refactor of buffer handling #27738

Merged

simonjayhawkins deleted the pandas-io-common.py branch August 5, 2019 09:39

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

TYPING: some type hints for pandas\io\common.py (pandas-dev#27598)

dd80e76

jorisvandenbossche mentioned this pull request Aug 21, 2019

Backport PR #27991 on branch 0.25.x (DataFrame html repr: also follow min_rows setting) #28044

Merged

simonjayhawkins mentioned this pull request Oct 7, 2019

Use pytest tmp file management fixtures instead of ensure_clean[_path|_dir] #26984

Closed

	f: TextIO, dialect: Type[csv.excel] = csv.excel, encoding: str = "utf-8", **kwds
	f: TextIO, dialect: Type[csv.Dialect] = csv.excel, encoding: str = "utf-8", **kwds

Uh oh!

TYPING: some type hints for pandas\io\common.py #27598

TYPING: some type hints for pandas\io\common.py #27598

Uh oh!

Conversation

simonjayhawkins commented Jul 25, 2019

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!

simonjayhawkins commented Jul 28, 2019

Uh oh!

simonjayhawkins commented Jul 30, 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!

jreback commented Aug 2, 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 Aug 2, 2019

Uh oh!

Uh oh!