Share the argument handling between ssd_hc() and ssd_hp() - #203
Draft
joethorley wants to merge 1 commit into
Draft
joethorley wants to merge 1 commit into
joethorley wants to merge 1 commit into
Conversation
`ssd_hc.fitdists()` and `ssd_hp.fitdists()` repeated the same `multi_est` and `ci_method = "weighted_arithmetic"` deprecation blocks and `chk_*()` sequences, and the `ssd_hp(proportion = FALSE)` soft deprecation appeared verbatim in `ssd_hp.fitdists()`, `ssd_hp.fitburrlioz()` and `ssd_hp_bcanz()`. Move them into three helpers next to `.hc_proportion()`, so each argument's validation and deprecation lives in one place and the escalations planned in #198 are a single change each. `lifecycle::deprecate_soft()` only warns when the deprecated function was called from the global environment or a test, and works that out from its calling frames, so the helpers pass the exported function's frame and its caller's frame explicitly. Without that the warnings vanished while `lifecycle::expect_deprecated()`, which forces verbosity, still passed; a test under `lifecycle_verbosity = "warning"` now guards it. The messages are byte identical to `dev` for all six deprecation paths. No snapshot changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This branch has not been deployed
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
ssd_hc.fitdists()andssd_hp.fitdists()shared 44 of their 70 to 80 body lines: the samemulti_estandci_method = "weighted_arithmetic"deprecation blocks and the samechk_*()sequences, then a call tohcp(). Thessd_hp(proportion = FALSE)soft deprecation appeared verbatim three times (ssd_hp.fitdists(),ssd_hp.fitburrlioz(),ssd_hp_bcanz()). The computation was already unified in thehcp*family; this PR unifies the argument handling too, in three internal helpers beside.hc_proportion()inR/hc.R:.hcp_est_method(est_method, multi_est, fun).hcp_ci_method(ci_method, fun).hp_proportion(proportion, deprecated, fun, id = NULL)funnames the calling function in the messages, so the text is unchanged per call site. Each argument's validation and deprecation now lives in one place, which makes the escalations planned in #198 a single change each.The one non-obvious part
lifecycle::deprecate_soft()only warns when the deprecated function was called from the global environment or from a test, and it determines that from its calling frames (envanduser_env). Moving the call into a helper made the helper the "deprecated function", whose caller is package code, and every soft deprecation went silent.lifecycle::expect_deprecated()did not notice because it forceslifecycle_verbosity = "warning", so the full suite stayed green through the broken intermediate state. The helpers therefore passenv = parent.frame()anduser_env = parent.frame(2), the exported function's frame and its caller's frame, and a new test underlifecycle_verbosity = "warning"exercises all five deprecation paths so a future move cannot silence them again.Verification
Deprecation output for six calls (
ssd_hp(f, 1),ssd_hp_bcanz(f, 1),ssd_hc(f, multi_est = TRUE),ssd_hp(f, 1, multi_est = FALSE), andci_method = "weighted_arithmetic"through bothssd_hc()andssd_hp()) is byte identical betweendevand this branch. No snapshot changed. Full suite 1408 passing, 0 failures;R CMD check0 errors, 0 warnings, 0 notes;air format --checkclean.Also adds a test that
ssd_hp()rejects an unknownest_methodorci_methodwith the samechk_subset()message asssd_hc();hcp()already did this internally, so it passed before and after, but it now documents the contract at the front door.