Use component names for plotting #1783
Conversation
There was a problem hiding this comment.
Thanks for this PR @sudrich, it looks good.
I had a minor suggestions for using only the label if the series only has a signel component. We could adapt the docstring for label accordingly.
| if label == "": | ||
| label_to_use = comp_name | ||
| else: | ||
| label_to_use = f"{label}_{comp_name}" |
There was a problem hiding this comment.
I agree with adding label as a prefix when there are multiple components.
For a single component, I think it's better to overwrite the name with label. WDYT?
| label_to_use = f"{label}_{comp_name}" | |
| label_to_use = label if len(self.components) == 1 else f"{label}_{comp_name}" |
There was a problem hiding this comment.
Fair enough. I've added it and changed the docstring accordingly.
There was a problem hiding this comment.
Looks good, thanks @sudrich 🚀
Just had a minor suggestion for rephrasing the docstring :)
Could you resolve the merge conflicts from the recent updates to the CHANGELOG?
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #1783 +/- ##
==========================================
- Coverage 94.19% 94.04% -0.16%
==========================================
Files 125 125
Lines 11505 11495 -10
==========================================
- Hits 10837 10810 -27
- Misses 668 685 +17
☔ View full report in Codecov by Sentry. |
sudrich
force-pushed
the
feature/reuse_original_labels_for_plot
branch
from
933e2aa
to
28f1939
Compare
Co-authored-by: Dennis Bader <dennis.bader@gmx.ch>
sudrich
force-pushed
the
feature/reuse_original_labels_for_plot
branch
from
28f1939
to
fab511e
Compare
Thanks for the review. I've addressed the outstanding issue and rebased to |
| if label == "": | ||
| label_to_use = comp_name | ||
| else: | ||
| label_to_use = f"{label}_{comp_name}" |
* Use component names for plotting * Add changelog entry * Use label directly for single component series * Adjust docstring to better reflect changes Co-authored-by: Dennis Bader <dennis.bader@gmx.ch> --------- Co-authored-by: Dennis Bader <dennis.bader@gmx.ch>
Summary
This PR changes
TimeSeries.plot()to be in line with its documentation.Example:
Imagine invoking
plot(label="prediction")on aTimeSerieswith the labels[A, B, C].Before the change, the labels are using an enumeration of the components:
[prediction_0, prediction_1, prediction_2].With the change, the labels are using the component names:
[prediction_A, prediction_B, prediction_C].Other Information
The change also allows adding a prefix for a series with a single component. This was not possible for some reason.
Example:
Imagine invoking
plot(label="prediction")on aTimeSerieswith the labels[A].Before the change, the label would just be ignored:
[A].With the change, the label is taken into account:
[prediction_A].This behaviour is still possible to recreate by calling
plot()orplot(label="").