CLN: annotations in core.apply #29477
Conversation
pandas/core/apply.py
Outdated
|
|
||
| @property | ||
| def axis(self) -> int: | ||
| raise AbstractMethodError(self) |
There was a problem hiding this comment.
can we not use @abc.abstractmethod going forward for new code?
There was a problem hiding this comment.
OK. It looks like for axis I can do just axis: int but for the others I have to use property+abc.abstractmethod.
Updated.
|
|
||
| def __init__( | ||
| self, | ||
| obj: "DataFrame", |
There was a problem hiding this comment.
should we consider making FrameApply generic if we want subclassed DataFrames to return same type?
There was a problem hiding this comment.
im not sure i understand the question. FrameApply is the base class for ColumnFrameApply and RowFrameApply
There was a problem hiding this comment.
I've not looked in detail but may not be relevant here.
Not sure how geopandas (or other downstream packages work), Just keeping in mind that, say GeoDataFrame, may return a GeoDataFrame with certain operations.
did start to type geopandas so I had a better idea, see geopandas/geopandas@master...simonjayhawkins:typing.
Note to self: Should try and get back to this.
| raise AbstractMethodError(self) | ||
|
|
||
| @property | ||
| def series_generator(self) -> Iterator["Series"]: |
There was a problem hiding this comment.
For return types, this could also be typed as Iterable. we've done this in a couple of places.
There was a problem hiding this comment.
isnt iterator more informative than iterable?
There was a problem hiding this comment.
for a return type it's nor relevant, so it's just a consistency thing.
|
updated the abstractmethod thing. didnt change Iterator-> Iterable because i still have a mild preference, but will if its a deal-breaker. |
|
cool |
make
apply_broadcasthave consistent signature.After this I plan to do a pass to make these classes much less stateful