Expose all settings in ydata-profiling ProfileReport #2921
Conversation
Signed-off-by: Eduardo Apolinario <[email protected]>
eapolinario
requested review from
wild-endeavor,
kumare3,
pingsutw,
cosmicBboy,
samhita-alla,
thomasjpfan and
Future-Outlier
as code owners
| @@ -53,12 +53,21 @@ class FrameProfilingRenderer: | |||
| Generate a ProfileReport based on a pandas DataFrame | |||
| """ | |||
|
|
|||
| def __init__(self, title: str = "Pandas Profiling Report"): | |||
| def __init__(self, title: Optional[str] = None): | |||
There was a problem hiding this comment.
API wise, there are now two places to set kwargs for ydata_profiling.ProfileReport, in __init__ (for only title) and in to_html.
By looking at the Renderable protocol, it looks like the design was to have configuration passed in __init__, because the to_html to restricted to one input:
https://github.com/flyteorg/flytekit/blob/master/flytekit/deck/renderer.py#L17-L23
There was a problem hiding this comment.
Yeah, I saw that, but I don't think we quite adhering to the protocol everywhere, for example the pandera renderer.
This is an indication that all invocations of to_html happen in user-code, so the protocol is "just" a convention? Can we make it more strict (and if so, I think we should add kwargs to the protocol).
Why are the changes needed?
ydata-profiling
ProfileReporthas a rich set of settings that should be exposed in flyte decks.What changes were proposed in this pull request?
Expose
kwargspassed directly to the constructor ofProfileReport.I set the minimal version of
ydata-profilingto2.4.0to support minimal-mode.How was this patch tested?
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Related to flyteorg/flyte#5993
Docs link