Multi yoy no label

[cdeline]

• Code changes are covered by tests
• Code changes have been evaluated for compatibility/integration with TrendAnalysis
• New functions added to __init__.py
• API.rst is up to date, along with other sphinx docs pages
• Example notebooks are rerun and differences in results scrutinized
• Updated changelog
• Update degradation and multi-YoY notebook example to remove label=
• Remove old degradation_timeseries_plot_old function cruft

[cdeline]

OK, so it looks like taking the median of every slope whose center-point falls within the moving window will re-create the original pd.rolling() approach for the standard (non-multi-yoy) case. Another change is that the new approach effectively labels the datapoints with label=center while the old approach is defaulting to label=right. This could be fixed if we wanted to go back to the original implementation.

[cdeline]

.. But with the multi-YoY output, taking the median of all slopes rather than a pointwise average will result in much more smoothing in the center of the timeseries (where you have more slopes available), but more variability at start and end of the timeseries when you're only averaging the 1-yr slopes.

[martin-springer]

.. But with the multi-YoY output, taking the median of all slopes rather than a pointwise average will result in much more smoothing in the center of the timeseries (where you have more slopes available), but more variability at start and end of the timeseries when you're only averaging the 1-yr slopes.

Agreed, with the multi-yoy approach the time series plot becomes less meaningful as it kind of converges to the overall system degradation rate in the center of the plot and that's kind of unrelated to the years shown in the x-labels. How about, for plotting purposes, we only use the 1-year slopes (slopes <1.5 year to account for leap years) for the whole plot? Would this give us a better representation of the system degradation along the time axis?

[cdeline]

We could leverage the rolling_days input parameter - like filter out slopes that are longer than > (rolling_days *1.1). Then you should get similar plots whether you use multi-yoy or regular.

[cdeline]

OK, I've added a filtering step where only slopes <= 2 years + 1 day (731 days) are included in the median. This helps to avoid over-smoothing of the multi-YoY from having too many slope values. I think this is a good compromise. I decided against making the filtering based on the width of the moving window because you could quickly filter out all valid slopes.

[mdeceglie]

would this give us the behavior we're after? cdeline#1

@cdeline @martin-springer let me know what you think.

[Copilot]

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 updates TrendAnalysis defaults to no longer pass label through yoy_kwargs.
• Extends YoY calc output metadata with calc_info['YoY_times'] and updates degradation_timeseries_plot to use timestamps/durations from that structure.
• Updates v3.2.0 changelog entries to reflect the new YoY_times output 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.

[cdeline]

Mike's PR was good. I do want to update the rolling median though to have the min_periods=rolling_days//4 instead of //2. I think this is a minor fix that we can include in this RdTools revision to make the time-series plot more resilient to small outages without losing fidelity. The API is still backwards compatible, the new plots will include more detail though if segments were previously excluded due to insufficient data length.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 9 changed files in this pull request and generated 8 comments.

[cdeline]

OK, good to go @martin-springer and @mdeceglie.

[mdeceglie]

I do want to update the code to have the min_periods=rolling_days//4 instead of //2. I think this is a minor fix that we can include here to make the time-series plot more resilient to small outages without losing fidelity.

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.

[martin-springer]

I do want to update the code to have the min_periods=rolling_days//4 instead of //2. I think this is a minor fix that we can include here to make the time-series plot more resilient to small outages without losing fidelity.

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.

Commit 15918a3 adds the new argument min_periods_divisor. If it's not set it defaults to 2 but raises a FutureWarning that the default will change to 4 in the next release. Documented with Issue #514