TYP: Add TypeVars to NDFrame #30613
TYP: Add TypeVars to NDFrame #30613
Conversation
topper-123
force-pushed
the
type_ndframe
branch
from
d01e6cc to
ff14fc2
Compare
| # TODO: Not sure if above is correct - need someone to confirm. | ||
| axis = self._get_axis_number(kwargs.pop("axis", self._stat_axis_name)) | ||
| if fill_method is None: | ||
| data = self | ||
| else: | ||
| data = self.fillna(method=fill_method, limit=limit, axis=axis) | ||
| data = self._ensure_type( |
There was a problem hiding this comment.
why wouldn't you instead type .fillna?
There was a problem hiding this comment.
.fillna is aready typed as Optional[FrameOrSeries]. The problem here is that data is typed to FrameORSeries and it can't accept reassigning type, per mypy's rules. the alternative to the above would be:
filled = self.fillna(method=fill_method, axis=axis, limit=limit)
assert isinstance(filled, type(self))
data = filledwhich gets repetive with a code base the size of Pandas', IMO.
There was a problem hiding this comment.
Are you saying the Optional is throwing it off or something else?
There was a problem hiding this comment.
Yes, but by design from the mypy folks: Mypy complains if a parameter has its type redefined to a different/broader type.
So if ‘data’ has been defined as a type DataFrame, mypy will complain if we try to redefine ‘data’ to be Optional[DataFrame].
There was a problem hiding this comment.
oh I c, since we have inplace args the return type must be Optional. this is super annoying. Let's deprecate inplace!
yeah ok i understand why you need this.
topper-123
force-pushed
the
type_ndframe
branch
from
ff14fc2 to
f402fb4
Compare
topper-123
force-pushed
the
type_ndframe
branch
from
f402fb4 to
6fd326d
Compare
|
thanks @topper-123 |
|
|
||
| Used by type checkers. | ||
| """ | ||
| assert isinstance(obj, type(self)), type(obj) |
There was a problem hiding this comment.
This line breaks any code that creates a subclass of the pandas.core.frame.DataFrame. issubclass should be used instead.
There was a problem hiding this comment.
Yes, see also #31925 (maybe repeat your comment there). We should fix this for 1.0.2
…1925) Commit pandas-dev/pandas@6fd326d in pull request pandas-dev#30613 added `_ensure_type`, which utilizes `isinstance`. However, it is reasonable to assume that someone may want to create a DataFrame subclass. Therefore, `_ensure_type` should use `issubclass`.
…1925) Commit pandas-dev/pandas@6fd326d in pull request pandas-dev#30613 added `_ensure_type`, which utilizes `isinstance`. However, it is reasonable to assume that someone may want to create a DataFrame subclass. Therefore, `_ensure_type` should use `issubclass`.
Adding TypeVars to NDFrame methods that don't return optional values. This ensures that e.g.
(DataFrame|Series).astypeand(DataFrame|Series).copyhave a known return type, which is nice.Also adds
PandasObject._ensure_type, which is a method used to solve the problem with optional return values, but where we actually know the return type. This is a helper method to help with the tediousness of optional return values experienced in #30565.