Release 3.2.0#487
Conversation
* add keyword 'label' to degradation_timeseries_plot, enabling 'left' and 'center' labeling options. * Update changelog, add pytests, update sphinx documentation * fix flake8 grumbles * update pytests to include axes limits * fix flake8 grumbles * add 'label' input option to `degradation_year_on_year`. Fixes #459 * add pytests and update changelog. * flake8 grumbles * Minor updates to setup.py (constrain scipy<1.16) and refactor degradation_test * Custom fix for Pandas < 2.0.0 which can't average two columns of timestamps. * flake8 grumbles * keep TZ-aware timestamps. Update pytests to specifically test _avg_timestamp_old_Pandas * flake8 grumbles * try to UTC localize the pytest... * Add .asfreq() to get pytests to agree * switch to calendar.timegm to hopefully remove TZ issues.. * regardless of uncertainty_method, return calc_info{'YoY_values') * update _right dt labels to correct _left labels in degradation_year_on_year * update _avg_timestamp_old_Pandas to allow for numeric index instead of timestamp * add left label option to degradation_year_on_year * update degradation_year_on_year, index set to either left, center or right. Consistent with #394 - multi_yoy * update return for default = none uncertainty option * degradation_year_on_year - go back to single return when uncertainty_value = None to avoid breaking pytests. * add multi-year aggregation of slopes in degradation_year_on_year * add multi_yoy kwarg in degradation_year_on_year to toggle the multi-YoY function. * update plotting for detailed=True, allow usage_of_points > 2 * flake8 grumbles * update plotting detailed=True for (even) and (odd) number of points coloring * To allow multi_yoy=True in plotting.degradation_timeseries_plot, resample.mean() the YoY_values. * flake8 grumbles * Add warning to degradation_timeseries_plot when multi_YoY=True * update to warning message in plotting.degradation_timeseries_plot * fix flake8 grumbles * nbval fixes from qnguyen345-bare_except_error * Add pandas 3.0 futurewarning handling * Try again to solve pandas3.0 futurewarning * attempt 3 to fix nbval * Add infer_objects to remove futurewarning * minor inline comment update * update plotting tests to be relative value, update ordering of module import in plotting.py, per Copilot review. * update inline comments and whatsnew docs * Clean up inline comments per Copilot review * added multi-YoY pytest - still need to catch warnings * Add a warnings.catch_warnings to the plotting pytest * flake8 grumbles * add multi-YoY=True pytest * updated changelog * update changelog * implement copilot suggestions * linting * use s instead of ns for pandas 3 compatibility * update pandas version comparison * set matplotlib non-gui backend for tests * set dtype resolution based on pandas version * linting import order * boost degradation.py test coverage * update changelog * exclude coverage reports with suffixes * Update rdtools/degradation.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * remove label=None handling, rely on default 'right' behavior * refactor dt_center tz handling for old pandas * simplify _avg_timestamp_old_Pandas * degradation_timeseries_plot: change rolling median min_periods to rolling_days / 4. * remove degradation_timeseries_plot(label=) and just default to center=True * update sensor_analysis() and clearsky_analysis() docstrings to discuss passing `label=right` kwargs * flake8 updates * Initial commit - multi-YoY notebook * pretty-print notebook dataframes with tabulate. * update notebook requirements to silence pandas warnings * add multi-yoy nb to tutorials * fix nblink path * Change the yoy_values index to be named 'dt'. Add new illustrations at the end of the multi-YoY notebook. --------- Co-authored-by: Michael Deceglie <mdeceglie@users.noreply.github.com> Co-authored-by: martin-springer <martinspringer.ms@gmail.com> Co-authored-by: Martin Springer <97482055+martin-springer@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* add keyword 'label' to degradation_timeseries_plot, enabling 'left' and 'center' labeling options. * Update changelog, add pytests, update sphinx documentation * fix flake8 grumbles * update pytests to include axes limits * fix flake8 grumbles * add 'label' input option to `degradation_year_on_year`. Fixes #459 * add pytests and update changelog. * flake8 grumbles * Minor updates to setup.py (constrain scipy<1.16) and refactor degradation_test * Custom fix for Pandas < 2.0.0 which can't average two columns of timestamps. * flake8 grumbles * keep TZ-aware timestamps. Update pytests to specifically test _avg_timestamp_old_Pandas * flake8 grumbles * try to UTC localize the pytest... * Add .asfreq() to get pytests to agree * switch to calendar.timegm to hopefully remove TZ issues.. * regardless of uncertainty_method, return calc_info{'YoY_values') * update _right dt labels to correct _left labels in degradation_year_on_year * update _avg_timestamp_old_Pandas to allow for numeric index instead of timestamp * add left label option to degradation_year_on_year * update degradation_year_on_year, index set to either left, center or right. Consistent with #394 - multi_yoy * update return for default = none uncertainty option * degradation_year_on_year - go back to single return when uncertainty_value = None to avoid breaking pytests. * add multi-year aggregation of slopes in degradation_year_on_year * add multi_yoy kwarg in degradation_year_on_year to toggle the multi-YoY function. * update plotting for detailed=True, allow usage_of_points > 2 * flake8 grumbles * update plotting detailed=True for (even) and (odd) number of points coloring * To allow multi_yoy=True in plotting.degradation_timeseries_plot, resample.mean() the YoY_values. * flake8 grumbles * Add warning to degradation_timeseries_plot when multi_YoY=True * update to warning message in plotting.degradation_timeseries_plot * fix flake8 grumbles * nbval fixes from qnguyen345-bare_except_error * Add pandas 3.0 futurewarning handling * Try again to solve pandas3.0 futurewarning * attempt 3 to fix nbval * Add infer_objects to remove futurewarning * minor inline comment update * update plotting tests to be relative value, update ordering of module import in plotting.py, per Copilot review. * update inline comments and whatsnew docs * Clean up inline comments per Copilot review * added multi-YoY pytest - still need to catch warnings * Add a warnings.catch_warnings to the plotting pytest * flake8 grumbles * add multi-YoY=True pytest * updated changelog * update changelog * implement copilot suggestions * linting * use s instead of ns for pandas 3 compatibility * update pandas version comparison * set matplotlib non-gui backend for tests * set dtype resolution based on pandas version * linting import order * boost degradation.py test coverage * add tests for error handling in analysis_chain * update changelog * update changelog * exclude coverage reports with suffixes * Update rdtools/degradation.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * remove label=None handling, rely on default 'right' behavior * refactor dt_center tz handling for old pandas * simplify _avg_timestamp_old_Pandas * degradation_timeseries_plot: change rolling median min_periods to rolling_days / 4. * remove degradation_timeseries_plot(label=) and just default to center=True * update sensor_analysis() and clearsky_analysis() docstrings to discuss passing `label=right` kwargs * flake8 updates * Initial commit - multi-YoY notebook * pretty-print notebook dataframes with tabulate. * update notebook requirements to silence pandas warnings * add multi-yoy nb to tutorials * fix nblink path * Change the yoy_values index to be named 'dt'. Add new illustrations at the end of the multi-YoY notebook. * clean up pending changelog --------- Co-authored-by: cdeline <chris.deline@nrel.gov> Co-authored-by: Michael Deceglie <mdeceglie@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR aggregates changes for the v3.2.0 release, centered on expanding the year-on-year (YoY) degradation workflow (labeling + multi-YoY), improving degradation plotting behavior, and adding broader error-handling test coverage and documentation updates.
Changes:
- Extend
degradation_year_on_yearwithlabel(right|center|left),multi_yoy, and newcalc_info['YoY_times']output, plus updatedusage_of_pointshandling for multi-YoY. - Update
degradation_timeseries_plotrolling behavior (centered rolling median, lowermin_periods) and add fallback handling for multi-YoY/duplicate-index data. - Add/expand tests, docs, notebooks, CI notebook validation, and release changelog for v3.2.0.
Reviewed changes
Copilot reviewed 15 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
rdtools/degradation.py |
Adds label/multi_yoy support, YoY_times, and updates usage_of_points computation. |
rdtools/plotting.py |
Adjusts degradation timeseries rolling behavior and adds multi-YoY plotting fallback/warning. |
rdtools/analysis_chains.py |
Changes default yoy_kwargs to explicitly default label="right" in analysis chains. |
rdtools/test/plotting_test.py |
Adds coverage for label alignment behavior and multi-YoY warning path in plotting. |
rdtools/test/degradation_test.py |
Adds tests for center labeling, multi-YoY, pandas-compat helper, and more error-path coverage. |
rdtools/test/conftest.py |
Sets matplotlib backend to Agg before importing rdtools to stabilize headless CI. |
rdtools/test/analysis_chains_test.py |
Adds tests for setter validation and additional error paths. |
docs/sphinx/source/examples/Multi-year_on_year_example.nblink |
Adds new example notebook link for multi-YoY/label usage. |
docs/sphinx/source/examples.rst |
Registers the new example in Sphinx examples. |
docs/sphinx/source/changelog/v3.2.0.rst |
Introduces v3.2.0 changelog entry. |
docs/sphinx/source/changelog.rst |
Includes pending + v3.2.0 changelog files. |
docs/notebook_requirements.txt |
Updates notebook dependency pins (incl. numexpr, tabulate). |
docs/TrendAnalysis_example*.ipynb |
Removes trailing semicolons/cleans notebook cells. |
.github/workflows/nbval.yaml |
Fixes nbval sanitize CLI usage. |
.gitignore |
Adds .coverage.* and fixes comment typo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@martin-springer : I'm getting setup.py deprecation messages when I install. Would this be the time to switch to a pyproject.toml? |
@cdeline - yes, we should do that. I'll open a PR and make the changes. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #487 +/- ##
==========================================
+ Coverage 96.27% 96.87% +0.60%
==========================================
Files 12 12
Lines 2280 2339 +59
==========================================
+ Hits 2195 2266 +71
+ Misses 85 73 -12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…lti-YoY detection (#490) * Initial plan * Replace broad except ValueError with explicit duplicate-index check in degradation_timeseries_plot Co-authored-by: cdeline <23244088+cdeline@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: cdeline <23244088+cdeline@users.noreply.github.com>
|
With |
| if results_values.index.has_duplicates: | ||
| # this occurs with degradation_year_on_year(multi_yoy=True). resample to daily mean | ||
| warnings.warn( | ||
| "Input `yoy_info['YoY_values']` appears to have multiple annual " | ||
| "slopes per day, which is the case if " | ||
| "degradation_year_on_year(multi_yoy=True). " | ||
| "Proceeding to plot with a daily mean which will average out the " | ||
| "time-series trend. Recommend re-running with " | ||
| "degradation_year_on_year(multi_yoy=False)." | ||
| ) | ||
| roller = results_values.resample('D').mean().rolling(f'{rolling_days}d', | ||
| min_periods=rolling_days//4, | ||
| center=True) |
There was a problem hiding this comment.
I'm unclear on why this is needed. At the end of the day we're plotting a median of all the relevant slopes within rolling_days why not just throw the multi-yoy slopes in with the rest of the sample instead of taking an intermediate mean?
There was a problem hiding this comment.
This may be a limitation of the pd.rolling function which I believe requires a unique timeindex to operate on. I can investigate options here.
There was a problem hiding this comment.
I'm opening a side branch 'multi_YoY_no_label' to refactor around removing the label= kwarg in degradation_year_on_year and re-write degradation_timeseries_plot() to plot the median of all slopes that have either their starting or ending point within the rolling_days window.
The main thing that |
|
Which label is the best one to use, especially when multi-yoy is in play? Center? |
Yes I would definitely recommend center-labeling of the 'YoY_values' timeseries, particularly with multi-yoy analysis. If we get rid of the |
|
Well we can't change the default behavior of I suggest that My concern here is twofold:
|
* updated degradation_timeseries_plot as version2 for now * fix flake8 grumbles * avoid pandas error "ValueError: Other Series must have a name" * Redo degradation_timeseries_plot - include in median if dt_center is within rolling window. * remove label= functionality. Update Multi-year_on_year_example.ipynb * Re-run Multi-year_on_year_example.ipynb, check for text changes. * update analysis_chains to remove `label=`. Update whatsnew * simplify multi-YoY timeseries plotting by filtering to slopes <= 2 years, removing warning. * update whatsnew * code cleanup * Revision of degradation_timeseries_plot * Bug fixes * Update notebooks to include `center=True` * Update nbval to include new notebook * add new kwarg to TrendAnalysis.plot_degradation_timeseries * Update plotting.py timeseries plot docstring. Also set pd.rolling(min_periods=rolling_days//4) * fix copilot documentation review recommendations; add plotting pytests * add min_periods_divisor argument * update pvdaq_system_4 url (#513) * update pvdaq_system_4 url * changelog * revert unnecessary changes in notebooks * update changelog * re-run notebooks --------- Co-authored-by: Michael Deceglie <Michael.Deceglie@nrel.gov> Co-authored-by: martin-springer <martin.springer@nrel.gov> Co-authored-by: Martin Springer <97482055+martin-springer@users.noreply.github.com>
mdeceglie
left a comment
There was a problem hiding this comment.
This is looking pretty close! In addition to my contextual comments, a few comments on the example notebooks:
- Change the links to the example data in the notebooks' markdown sections
- Let's add some more explanation to the TrendAnalysis example notebook
plot_degradation_timeseriespart. Something like: "In this case the fluctuations in apparent degradation rate are probably due to the soiling component of the signal. This example illustrates the pitfalls of attempting to look at how degradation rate evolves in time in the presence of a large uncorrected soiling signal. Nevertheless it illustrates the plotting functionality that can be useful in applications not affected by soiling or other reversible changes."
| Enhancements | ||
| ------------ | ||
| * :py:func:`~rdtools.degradation.degradation_year_on_year` has new parameter ``multi_yoy`` | ||
| (default False) to trigger multiple YoY degradation calculations similar to Hugo Quest et |
There was a problem hiding this comment.
Let's include a citation to the Quest paper
| @@ -0,0 +1,78 @@ | |||
| ************************* | |||
| v3.2.0 (X, X, 2026) | |||
There was a problem hiding this comment.
We need to add an explanation that the index of YoY_values has changed from a timestamp to integer and that the original behavior can be reconstructed with the help of calc_info['YoY_times']. Strictly speaking, I don't think this is backward compatible, but for this case I think it may be ok to keep it in a minor version update.
| @@ -0,0 +1,78 @@ | |||
| ************************* | |||
| v3.2.0 (X, X, 2026) | |||
| ************************* | |||
There was a problem hiding this comment.
Another even more in the weeds changes is that we have replaced NaNs in calc_info['usage_of_points'] with zeros. I don't know if this ever happens in practice but probably worth it to call out the edge case change.
| @@ -218,7 +224,7 @@ def degradation_year_on_year(energy_normalized, recenter=True, | |||
| degradation rate estimate | |||
| calc_info : dict | |||
There was a problem hiding this comment.
should add explanation of calc_info['YoY_times']
Summary
This PR collects the changes for the v3.2.0 release, including new multi-year-on-year degradation analysis features, plotting improvements, bug fixes, and expanded test coverage.
Enhancements
degradation_year_on_year—labelparameter (degradation_year_on_year should allow center-labeled YoY_values #459): Newlabel=parameter ('right','center', or'left') controls which edge of the YoY slope is used for labelingcalc_info['YoY_values']. Default is'right'(backward-compatible).degradation_year_on_year—multi_yoyparameter (Multi-YoY analysis based on Hugo Quest et al 2023 #394): Newmulti_yoy=parameter (defaultFalse) enables multi-year-on-year degradation calculations similar to Hugo Quest et al 2023, computing slopes over N × 365 day separations.degradation_year_on_year—YoY_timesoutput (degradation_year_on_year should allow center-labeled YoY_values #459):calc_info['YoY_times']now returns a DataFrame withdt_right,dt_center, anddt_leftcolumns for each slope.degradation_timeseries_plotimprovements (degradation_timeseries_plot should have different label= options #455): Rolling median is now centered (center=True) andmin_periodsreduced fromrolling_days//2torolling_days//4.degradation_timeseries_plotmulti-YoY support (Multi-YoY analysis based on Hugo Quest et al 2023 #394): Handlesmulti_yoy=Truedata by resampling overlapping YoY values to their daily mean (with a warning).degradation_summary_plotsparity coloring (Multi-YoY analysis based on Hugo Quest et al 2023 #394):detailed=Truemode now properly colors points by odd/even usage count, not just 0/1/2.sensor_analysis/clearsky_analysis: Now explicitly defaultyoy_kwargs={"label": "right"}.docs/Multi-year_on_year_example.ipynbdemonstrateslabel='center'andmulti_yoy=True(Multi-YoY analysis based on Hugo Quest et al 2023 #394).Bug Fixes
usage_of_pointscalculation indegradation_year_on_yearto properly handlemulti_yoy=Truemode with overlapping slopes (Multi-YoY analysis based on Hugo Quest et al 2023 #394).Maintenance
_avg_timestamp_old_Pandashelper for pandas <2.0 compatibility when calculating center labels.--sanitize-with→--nbval-sanitize-with).docs/notebook_requirements.txtto requirenumexpr>=2.10.2andtabulate>=0.9.0..coverage.*pattern to.gitignore.Testing
analysis_chains:filter_paramsvalidation,clearsky_rescale_index_mismatch,poa_filter_without_poa,tcell_filter_without_temperature,hour_angle_filter_without_location,clearsky_filter_without_poa,degradation_timeseries_plot_invalid_case.degradation:classical_decompositionmissing/irregular data,year_on_yearcircular block validation, no valid pairs,_mk_testedge cases.multi_yoy=Trueindegradation_year_on_year.degradation_timeseries_plotcoveringlabel='center',label='left', multi-YoY duplicate index, andKeyErrorpath.Agginconftest.pyto avoid tkinter issues.Checklist
__init__.py