feat: Add plot_params to allow custom figsize (#68)#69
feat: Add plot_params to allow custom figsize (#68)#69alexbthundiyil-spec wants to merge 3 commits intoRussellSB:developfrom
Conversation
|
Hey @alexbthundiyil-spec ! Thanks for the PR! For completeness, would you mind adding a new module under tests/test_plotting/custom to reflect this? Can add a simple test that the image changes in figsize (and/or title) as its supposed to. I use P.S. I have also made minor edits to the docstring to resolve a merge conflict, and align with a slightly new format I've tested looks nicer on the recently deployed docs site. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
alexbthundiyil-spec
force-pushed
the
feature/issue-68-figsize
branch
from
e415531 to
063bb50
Compare
|
I've gone ahead and added the requested |
RussellSB
left a comment
RussellSB
left a comment
There was a problem hiding this comment.
Thank you very much for adding a test! I've had a look and added some comments. At the moment title doesn't update since it cant be passed through 'plt.subplots()'.
Can either:
- Add a workaround to handle the non subplot properties
- Limit this PR to focus on figsize as a foundation, and address nitpicking comments to make this more clear
I'm happy with either.
|
|
||
| plot_params = { | ||
| 'figsize': (16, 8), | ||
| 'title': "Custom Plot Title" |
There was a problem hiding this comment.
Can remove title it turns out, it can't be passed to subplots().
| - **is_abrupt_padded** (`bool`): Whether to pad abrupt transitions between segments. Defaults to `False`. | ||
| - **abrupt_padding** (`int`): Number of days to pad around abrupt transitions. Only referenced when `is_abrupt_padded` is `True`. Defaults to `28`. | ||
| plot_params (dict, optional): | ||
| Dictionary of plotting parameters to pass to `plot_pytrendy`. Supported keys: |
There was a problem hiding this comment.
Specify in comment to pass to plt.subplots() in plot_pytrendy. In case user is interested in trying other args as long as it's applicable to subplots call.
| ax.set_title("PyTrendy Detection", fontsize=20) | ||
| ax.set_xlabel("Date") | ||
| ax.set_ylabel("Value") |
There was a problem hiding this comment.
Title related properties would need to be passed here rather than subplots.
I think it could be useful to make this customisable alongside figsize, however they would need to be identified and passed into here.
Up to you. Following other comments, I'm also fine if we just limit this PR to figsize as a foundation, then can build onto it with other PRs.
2 participants
Fixes #68 by mapping a plot_params argument to the internal matplotlib subplots.