TYP: check_untyped_defs io.sas.sasreader #30659
Conversation
pandas/io/sas/sasreader.py
Outdated
| @@ -63,14 +71,15 @@ def read_sas( | |||
| else: | |||
| raise ValueError("unable to infer format of SAS file") | |||
|
|
|||
| reader: Union["XportReader", "SAS7BDATReader"] | |||
There was a problem hiding this comment.
Do these have a shared base class that is directly importable? Seems like this requires a bit of import gymnastics not sure worth it
There was a problem hiding this comment.
they share abc.Iterable ...
pandas\io\sas\sasreader.py:93: error: "Iterator[Any]" has no attribute "read"
pandas\io\sas\sasreader.py:94: error: "Iterator[Any]" has no attribute "close"
many options:
Use protocol (python 3.8) from typing_extensions
define abc base class with read and close methods (add comment to use Protocol from py3.8)
use dynamically typed code, i.e. reader: Any
use ignore
leave as is with import gymnastics and Union.
we normally try to avoid most of these.
There was a problem hiding this comment.
define abc base class with read and close methods (add comment to use Protocol from py3.8)
gone with this option.
pandas/io/sas/sasreader.py
Outdated
|
|
||
| def read_sas( | ||
| filepath_or_buffer, | ||
| format=None, | ||
| filepath_or_buffer: FilePathOrBuffer[AnyStr], |
There was a problem hiding this comment.
maybe we should change the definition in _typing?
There was a problem hiding this comment.
can you elaborate?
There was a problem hiding this comment.
FilePathOrBuffer[AnyStr] should be in _typing instead of just the not parametric FilePathOrBuffer
There was a problem hiding this comment.
for some read/write methods only a string buffer (or bytes buffer) is allowed, i.e. to_html and we need to annotate as FilePathOrBuffer[str] (or FilePathOrBuffer[bytes]) otherwise we get mypy errors.
There was a problem hiding this comment.
that’s a very limited case compared to the more general which accepts strings and buffers
these should be distinct / parametrized
not just in this PR - this is my point
There was a problem hiding this comment.
not sure I understand, AnyStr is a TypeVar and therefore parametrized https://docs.python.org/3/library/typing.html#typing.AnyStr
There was a problem hiding this comment.
so, you are adding this here, rather than as a generailized soln in _typing. I would think it would be better there than adding to each of the IO readers.
There was a problem hiding this comment.
i reverted these additions for now to keep this PR focused on the untyped_defs error, #30659 (comment)
pandas/io/sas/sasreader.py
Outdated
| from pandas.io.common import stringify_path | ||
|
|
||
| if TYPE_CHECKING: | ||
| from pandas.io.sas.sas_xport import XportReader # noqa: F401 |
There was a problem hiding this comment.
acutally i think you can just import both readers at the top and call it a day.
|
rebase as well |
|
need to sort out some import issues. closing to reduce queue for now |
This reverts commit 576c081.
|
@simonjayhawkins if you can merge master and merge on green. |
pandas\io\sas\sasreader.py:75: error: Incompatible types in assignment (expression has type "SAS7BDATReader", variable has type "XportReader")