Skip to content

Fix broad except ValueError in degradation_timeseries_plot for multi-YoY detection#490

Merged
cdeline merged 2 commits intodevelopmentfrom
copilot/sub-pr-487-again
Mar 6, 2026
Merged

Fix broad except ValueError in degradation_timeseries_plot for multi-YoY detection#490
cdeline merged 2 commits intodevelopmentfrom
copilot/sub-pr-487-again

Conversation

Copy link
Contributor

Copilot AI commented Mar 5, 2026

The multi-YoY duplicate-timestamp case was detected by catching a broad ValueError from .rolling(), which also fires for non-monotonic indexes — masking unrelated errors and potentially failing if the input is unsorted.

Changes

  • rdtools/plotting.pydegradation_timeseries_plot:
    • Call results_values.sort_index() before rolling so unsorted datetime indexes never raise ValueError
    • Replace try/except ValueError with explicit results_values.index.has_duplicates check; warning and daily resample only trigger when duplicate timestamps actually exist
# Before: broad exception catch
try:
    roller = results_values.rolling(...)
except ValueError:
    warnings.warn("... multi_yoy ...")
    roller = results_values.resample('D').mean().rolling(...)

# After: explicit duplicate detection
results_values = results_values.sort_index()
if results_values.index.has_duplicates:
    warnings.warn("... multi_yoy ...")
    roller = results_values.resample('D').mean().rolling(...)
else:
    roller = results_values.rolling(...)

Checklist

  • 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

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…n degradation_timeseries_plot

Co-authored-by: cdeline <23244088+cdeline@users.noreply.github.com>
Copilot AI changed the title [WIP] Address feedback on Release 3.2.0 PR Fix broad except ValueError in degradation_timeseries_plot for multi-YoY detection Mar 5, 2026
@codecov-commenter
Copy link

codecov-commenter commented Mar 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.90%. Comparing base (1476f02) to head (ceb41f1).
⚠️ Report is 2 commits behind head on development.

Additional details and impacted files
@@             Coverage Diff              @@
##           development     #490   +/-   ##
============================================
  Coverage        96.90%   96.90%           
============================================
  Files               12       12           
  Lines             2328     2329    +1     
============================================
+ Hits              2256     2257    +1     
  Misses              72       72           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cdeline cdeline marked this pull request as ready for review March 6, 2026 18:06
@cdeline cdeline self-requested a review March 6, 2026 18:06
Copy link
Collaborator

@cdeline cdeline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tested in the multi-YoY notebook and this explicit check for duplicate indices works.

@cdeline cdeline merged commit 3f773bf into development Mar 6, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants