Add check.rwl() and enforce rwl class uniformly across public functions#37
Open
AndyBunn wants to merge 4 commits into
Open
Add check.rwl() and enforce rwl class uniformly across public functions#37AndyBunn wants to merge 4 commits into
AndyBunn wants to merge 4 commits into
Conversation
Previously, check.rwl() warned before attempting coercion, so a
structural failure (e.g. non-consecutive years) produced a confusing
pair of messages in reverse causal order, with an internal function
name (as.rwl) in the error.
Now coercion is attempted first inside tryCatch. On failure, a single
stop() names the class problem and the structural reason together.
The warning only fires on success, confirming coercion worked.
Before:
Error in as.rwl(rwl): 'x' must have consecutive integers as row names (years)
In addition: Warning message: 'rwl' is not class "rwl". Attempting to coerce.
After:
Error: 'rwl' is not class "rwl" and coercion failed: 'x' must have
consecutive integers as row names (years)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ages sgc() takes ring-width data (rwl), not a generic object — rename the argument from x to rwl for consistency with the rest of the package, and let check.rwl() validate it. chron() and chron.ars() take detrended ring-width indices (rwi), not raw ring widths. Rename x→rwi to make the expected input explicit. Remove the check.rwl() calls added in the previous commit: rwi is not an rwl and enforcing that class would fire a spurious coercion warning on every normal detrend() → chron() workflow. Update \usage, \arguments, and \value in the corresponding man pages. The \seealso pointer in chron.Rd and chron.ars.Rd is corrected from read.rwl to detrend, since detrend is what actually produces rwi. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers happy path, coercion, structural failures, internal NA detection, and all as.rwl() error conditions (17 tests total). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents check.rwl(), as.rwl() numeric column check, uniform rwl validation across 19 public functions, argument renames in sgc/chron/ chron.ars, and the new test suite additions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
check.rwl(), a new exported validation/coercion function that provides a single, consistent entry point for rwl validation across the package. If the input is not classrwl, coercion is attempted viaas.rwl(); on success a warning is issued; on failure a single informative error names both the class problem and the structural reason coercion failed. After class validation,check.rwl()also warns about internal NAs and names the affected series, directing the user tofill.internal.NA().as.rwl()that was previously missing.is.data.frame()/ warning-only class checks in 19 public functions (bai.in,bai.out,ccf.series.rwl,cms,common.interval,corr.rwl.seg,corr.series.seg,detrend,i.detrend,interseries.cor,pointer,rcs,rwl.stats,seg.plot,series.rwl.plot,spag.plot,ssf,strip.rwl,xdate.floater) with a singlecheck.rwl()call.x → rwlinsgc()andx → rwiinchron()andchron.ars()to correctly reflect what each function expects; updates man pages accordingly.testthattests coveringcheck.rwl()andas.rwl().Test plan
devtools::test('.')passes with FAIL 0 (currently FAIL 0 | WARN 37 | SKIP 0 | PASS 454)check.rwl()on a proper rwl is silent and returns input unchangedcheck.rwl()on a plain data.frame warns "Coerced successfully" and returns an rwlcheck.rwl()on structurally bad input (non-consecutive years, non-numeric columns, non-data.frame) stops with a single message describing both the class problem and the structural causecheck.rwl()on an rwl with internal NAs warns and names the affected serieschron(rwi, ...)andchron.ars(rwi, ...)work normally — no spurious "not class rwl" warnings from the detrend → chron workflow