TYP: fix a few types #54976
TYP: fix a few types #54976
Conversation
pandas/__init__.py
Outdated
| @@ -110,6 +110,7 @@ | |||
|
|
|||
| from pandas.core.dtypes.dtypes import SparseDtype | |||
|
|
|||
| from pandas import util | |||
There was a problem hiding this comment.
For https://pandas.pydata.org/docs/reference/api/pandas.util.hash_pandas_object.html and other functions
There was a problem hiding this comment.
I think there are other methods in this module that are not meant to be public though: capitalize_first_letter, cache_readonly, Substitution, Appender
There was a problem hiding this comment.
Okay, I think we could do __all__ = [all_classes_functions_in_the_docs] (in utils/__init__) to account for that. And/or we could deprecate it from utils and put it as part of a different namespace.
There was a problem hiding this comment.
__all__ only changes behavior of from foo import *, no? What do we think of moving these functions (can we make a list of them?) to the top namespace?
Does this hold up other aspects of this PR? If not, I would suggest moving to a separate PR.
There was a problem hiding this comment.
Happy to remove this change from this PR. I will open an issue to first discuss how this should be handled.
__all__ affects from pandas.utils import * but (more importantly) type checkers, use it (next to other rules) to determine which symbols are meant to be public: if __all__ is present, only those symbols listed in it are considered public.
There was a problem hiding this comment.
if
__all__is present, only those symbols listed in it are considered public.
This doesn't seem to be the case with mypy:
# test/__init__.py
x = 5
y = 6
__all__ = ['y']
# bar.py
import test
print(test.x)
print(test.y)
mypy bar.py reports no issues for me.
There was a problem hiding this comment.
I must have misremembered that (pyright also allows that).
Pyright describes how it handles it: https://github.com/microsoft/pyright/blob/main/docs/typed-libraries.md#library-interface
| @@ -1926,11 +1926,17 @@ def to_dict( | |||
| self, | |||
| orient: Literal["dict", "list", "series", "split", "tight", "index"] = ..., | |||
| into: type[dict] = ..., | |||
| index: bool = ..., | |||
There was a problem hiding this comment.
Present in the implementation but missing in the overloads
| @@ -11711,6 +11717,7 @@ def quantile( | |||
| axis: Axis = ..., | |||
| numeric_only: bool = ..., | |||
| interpolation: QuantileInterpolation = ..., | |||
| method: Literal["single", "table"] = ..., | |||
There was a problem hiding this comment.
Present in the implementation but missing in the overloads
pandas/io/parsers/readers.py
Outdated
| kwds["iterator"] = iterator | ||
| kwds["chunksize"] = chunksize |
There was a problem hiding this comment.
Nit: Would it be better to do this down on L1462 with the other assignments?
pandas/__init__.py
Outdated
| @@ -110,6 +110,7 @@ | |||
|
|
|||
| from pandas.core.dtypes.dtypes import SparseDtype | |||
|
|
|||
| from pandas import util | |||
There was a problem hiding this comment.
__all__ only changes behavior of from foo import *, no? What do we think of moving these functions (can we make a list of them?) to the top namespace?
Does this hold up other aspects of this PR? If not, I would suggest moving to a separate PR.
| # error: Cannot instantiate abstract class 'ExcelWriter' with abstract | ||
| # attributes 'engine', 'save', 'supported_extensions' and 'write_cells' | ||
| writer = ExcelWriter( # type: ignore[abstract] | ||
| writer = ExcelWriter( |
There was a problem hiding this comment.
Won't this immediately raise in the try below?
There was a problem hiding this comment.
Ahh, no, because we overwrite __new__. I think this is okay.
|
@twoertwein - looks like isort is making a modification |
All green now :) |
|
Thanks @twoertwein |
* TYP: fix a few types * namespace test * read_fwf overloads * Revert "namespace test" This reverts commit 0f72079. * revert util and move kwds * isort
I fixed a few type issues discovered by using the pandas-stubs type tests with the pandas annotations (there are many errors).
I'm surprised that the two Excel-classes being marked as abstract wasn't detected by the CI, they are clearly meant to be instantiated, so they cannot be abstract.