New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modularize customization of time series plots #3009
Conversation
@ngupta23 Will add docstring and test for new util which customises plot. I currently moved this to |
There. are few other cases like below which also update x, y axes and traces , and are duplicated in two to three other plotting methods. pycaret/pycaret/internal/plots/time_series.py Lines 495 to 508 in b292c97
Would you prefer if this was all handled within the def _plot_update_trace_axes_dim_layout(fig, title, ncols, size, fig_defaults, fig_kwargs, show_legend):
with fig.batch_update():
for i in np.arange(1, ncols + 1):
fig.update_xaxes(title_text="Lags", row=1, col=i)
# Only on first column
fig.update_yaxes(title_text=plot.upper(), row=1, col=1)
fig.update_traces(marker={"size": 10})
return _plot_fig_dim_layout(fig, title, fig_defaults, fig_kwargs, show_legend)
|
@ryankarlos Can you check why the tests are failing? Thanks! |
please see comment in #3010 (comment) |
fig = _update_fig_dimensions( | ||
fig=fig, fig_kwargs=fig_kwargs, fig_defaults=fig_defaults | ||
) | ||
fig = _plot_fig_update(fig, title, fig_defaults, fig_kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should show_legend
be True here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. I think there may be a small change needed which I have noted. Please have a look. Thanks!
I do not think that those are that pervasive, so it is ok to leave it as is for now. |
Thanks @ngupta23 pushed new changes and merged master |
Related Issue or bug
Closes #2417
Describe the changes you've made
abstract out redundant code into util functions for customising plots which could be reused in multiple plotting methods/functions
Type of change
Checklist: