Multi yoy no label#498
Conversation
|
We could leverage the |
…ars, removing warning.
|
would this give us the behavior we're after? cdeline#1 @cdeline @martin-springer let me know what you think. |
There was a problem hiding this comment.
Pull request overview
This PR updates the year-on-year (YoY) degradation workflow to remove reliance on label=-based timestamping, adds explicit YoY timestamp metadata (YoY_times) to the degradation_year_on_year outputs, and refactors the degradation time-series plotting path to better support multi-YoY results.
Changes:
- Removes
label=-specific tests/fixtures and updatesTrendAnalysisdefaults to no longer passlabelthroughyoy_kwargs. - Extends YoY calc output metadata with
calc_info['YoY_times']and updatesdegradation_timeseries_plotto use timestamps/durations from that structure. - Updates v3.2.0 changelog entries to reflect the new
YoY_timesoutput and the revised multi-YoY time-series plotting approach.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| rdtools/test/plotting_test.py | Removes label-based coverage and updates multi-YoY handling assertions for degradation_timeseries_plot. |
| rdtools/test/degradation_test.py | Removes tests for now-removed/changed label='center' behavior. |
| rdtools/plotting.py | Refactors degradation_timeseries_plot to consume YoY_times and handle multi-YoY smoothing differently. |
| rdtools/degradation.py | Removes label documentation/logic and adjusts how YoY timestamps/indices are produced. |
| rdtools/analysis_chains.py | Removes default yoy_kwargs={"label": "right"} so chains don’t depend on the label parameter. |
| docs/sphinx/source/changelog/v3.2.0.rst | Updates changelog notes for YoY_times and the new multi-YoY plotting behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Mike's PR was good. I do want to update the rolling median though to have the |
…_periods=rolling_days//4)
|
OK, good to go @martin-springer and @mdeceglie. |
Since this would change the math relative to our latest release, I think the best path would be to make it an argument, we can change the default behavior in the next major version. |




__init__.pylabel=