REF: extract classes in pandas/core/describe.py #39186
Conversation
ivanovmg
changed the title
pandas/core/describe.py
Outdated
|
|
||
| def create_describer( |
There was a problem hiding this comment.
i would just inline this to above
pandas/core/describe.py
Outdated
|
|
||
| def __init__( | ||
| self, | ||
| series: "Series", |
There was a problem hiding this comment.
can you make this a common method in NDFrame (and changes series -> data) to match dataframe
There was a problem hiding this comment.
I will check that. But I guess it is going to violate Liskov principle.
There was a problem hiding this comment.
Looks like nothing is violated. It seems to work.
pandas/core/describe.py
Outdated
| frame : DataFrame | ||
| DataFrame to be described. | ||
| include : 'all', list-like of dtypes or None (default), optional | ||
| data : DataFrame |
There was a problem hiding this comment.
match series for data / datetime_is_numeric and add other args
| self, | ||
| frame: "DataFrame", | ||
| *, | ||
| include: Optional[Union[str, Sequence[str]]], |
There was a problem hiding this comment.
you can do super() for first 2 args
pandas/core/describe.py
Outdated
| @@ -376,3 +380,8 @@ def refine_percentiles(percentiles: Optional[Sequence[float]]) -> Sequence[float | |||
| raise ValueError("percentiles cannot contain duplicates") | |||
|
|
|||
| return unique_pcts | |||
|
|
|||
|
|
|||
| def validate_frame(frame: "DataFrame"): | |||
There was a problem hiding this comment.
a reason not to do this in the constructor? e.g. L148
There was a problem hiding this comment.
I though of simplifying the constructor.
Ideally I would create a method _initialize_frame, which will perform the validation under the hood and return the frame.
self.frame = self._initialize_frame(frame)
But that will be a static method, which you prefer not to use.
If you consider that this extraction is unnecessary, then I can definitely inline it into the constructor.
There was a problem hiding this comment.
Moved into the constructor.
|
thanks @ivanovmg |
Extract classes
SeriesDescriberandDataFrameDescriber.In the next steps I am going to enable strategy pattern and move corresponding describe functions to the concrete strategy classes (for each datatype).