Skip to content

Standardize PPC error handling by replacing stop() with rlang abort()#456

Merged
jgabry merged 3 commits intostan-dev:masterfrom
utkarshpawade:fix-stop-vs-abort-inconsistency
Mar 13, 2026
Merged

Standardize PPC error handling by replacing stop() with rlang abort()#456
jgabry merged 3 commits intostan-dev:masterfrom
utkarshpawade:fix-stop-vs-abort-inconsistency

Conversation

@utkarshpawade
Copy link
Contributor

Fixes #455

This PR replaces the remaining base stop() calls in PPC code paths with rlang::abort() to align with existing package conventions for structured error handling.

What changed

  • Updated validation errors in ppc_km_overlay() to use abort()
  • Updated interpolation validation errors in get_interpolation_values() to use abort()

Why

bayesplot already uses abort() in most places. This change removes inconsistency in error handling and improves maintainability without changing intended functionality.

Copilot AI review requested due to automatic review settings March 13, 2026 09:08
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR standardizes error handling in the PPC censoring and interpolation helpers by replacing the last remaining base stop() validations with rlang::abort(), aligning these code paths with the package’s prevailing structured error approach (Fixes #455).

Changes:

  • Replaced stop() with abort() for input validation in ppc_km_overlay().
  • Replaced stop() with abort() for interpolation feasibility checks in get_interpolation_values().
  • Documented the consistency change in NEWS.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
R/ppc-censoring.R Switches ppc_km_overlay() validation errors from stop() to abort().
R/helpers-ppc.R Switches interpolation validation errors in get_interpolation_values() from stop() to abort().
NEWS.md Adds a development NEWS entry describing the standardized error handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@jgabry jgabry left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good. Once #460 is merged we can update this branch and rerun the tests before merging.

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.55%. Comparing base (b1cb851) to head (95aa7cc).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #456   +/-   ##
=======================================
  Coverage   98.55%   98.55%           
=======================================
  Files          35       35           
  Lines        5864     5864           
=======================================
  Hits         5779     5779           
  Misses         85       85           

☔ 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.

@utkarshpawade
Copy link
Contributor Author

Thanks!

@jgabry jgabry merged commit 3f4e352 into stan-dev:master Mar 13, 2026
6 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.

Inconsistent error handling: use abort() instead of stop() in PPC helpers

4 participants