TYP: Enable mypy stricter typing for defs/calls #54271
Conversation
pandas/core/interchange/dataframe.py
Outdated
| @@ -82,7 +87,7 @@ def select_columns(self, indices) -> PandasDataFrameXchg: | |||
| self._df.iloc[:, indices], self._nan_as_null, self._allow_copy | |||
| ) | |||
|
|
|||
| def select_columns_by_name(self, names) -> PandasDataFrameXchg: | |||
| def select_columns_by_name(self, names: Sequence[str]) -> PandasDataFrameXchg: | |||
There was a problem hiding this comment.
This would allow str @Dr-Irv , does it work for a string at runtime?
There was a problem hiding this comment.
I don't think so. I think it's intended to accept a list-like of strings, but I just matched the base class here. Thoughts?
There was a problem hiding this comment.
If you only want to accept a list of strings, you have to use list[str]. If you use Sequence[str] then an argument of "abc" would match
pandas/io/gbq.py
Outdated
| @@ -215,7 +215,7 @@ def to_gbq( | |||
| table_schema: list[dict[str, str]] | None = None, | |||
| location: str | None = None, | |||
| progress_bar: bool = True, | |||
| credentials=None, | |||
| credentials: Any = None, | |||
There was a problem hiding this comment.
The documentation says, it needs to be google.auth.credentials.Credentials | None
There was a problem hiding this comment.
(but if the CI doesn't install the package, there is not much benefit of importing it in a TYPE_CHECKING block.)
There was a problem hiding this comment.
Thank you for enforcing type annotations @mroeschke !
I left a few suggestions to avoid Any but would also be fine to merge already.
cc @Dr-Irv
| @@ -87,7 +87,7 @@ def select_columns(self, indices: Sequence[int]) -> PandasDataFrameXchg: | |||
| self._df.iloc[:, indices], self._nan_as_null, self._allow_copy | |||
| ) | |||
|
|
|||
| def select_columns_by_name(self, names: Sequence[str]) -> PandasDataFrameXchg: | |||
| def select_columns_by_name(self, names: list[str]) -> PandasDataFrameXchg: # type: ignore[override] # noqa: E501 | |||
There was a problem hiding this comment.
This is pretty interesting that you have to do the override. In some sense, the method DataFrameXchg.select_columns_by_name() is wrong when it says names is Sequence[str], because probably the people who wrote that aren't aware that a pure str will match.
There was a problem hiding this comment.
Yeah this might be an oversight on the original spec. I opened an issue regarding this data-apis/dataframe-api#212
No description provided.