Add validation to ensure es_type parameter is only used with effectsize = "boot"#577
Conversation
Co-authored-by: rempsyc <13123390+rempsyc@users.noreply.github.com>
Co-authored-by: rempsyc <13123390+rempsyc@users.noreply.github.com>
es_type parameter when effectsize == "boot"es_type parameter is only used with effectsize = "boot"
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds validation to ensure that the es_type parameter can only be used when effectsize = "boot" in estimate_contrasts(), addressing issue #575. The change prevents silent parameter ignoring and provides clear error messages when the parameter is misused.
Key changes:
- Added validation logic using
match.call()to detect explicites_typeusage - Added comprehensive test coverage for both error and success cases
- Fixed a spelling error in documentation from "representiv" to "representative"
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| R/estimate_contrasts.R | Added validation logic to check es_type usage with appropriate effectsize values |
| tests/testthat/test-estimate_contrasts_effectsize.R | Added comprehensive test suite and reformatted existing tests for better readability |
| man/estimate_means.Rd | Fixed spelling error in documentation |
| inst/WORDLIST | Updated word list for spell checking |
Comments suppressed due to low confidence (1)
man/estimate_means.Rd:1
- Corrected spelling of 'representiv' to 'representative'.
% Generated by roxygen2: do not edit by hand
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| } else if (effectsize != "boot") { | ||
| insight::format_error("`es_type` can only be used when `effectsize = \"boot\"`.") | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we should move these checks into the function that computes the effect sizes, .estimate_contrasts_effectsize().
There was a problem hiding this comment.
But then we have a minor problem with the default effectsize = NULL (with es_type still specified) because then the checks get skipped and people will not see the error and therefore not understand why es_type is being ignored (or perceived as incorrect values):
modelbased/R/estimate_contrasts.R
Lines 366 to 376 in f2ba8b3
There's another area of confusion. If people are not allowed to manually specify es_type = "cohens.d" and be faced with the error, they might be confused as to why the defaultis es_type = "cohens.d" nonetheless? So if we change the default to es_type = NULL, then we possibly resolve this ambiguity and also the above problem (so checks can stay within .estimate_contrasts_effectsize()).
Summary
This PR adds validation to ensure that the
es_typeparameter can only be used wheneffectsize = "boot"inestimate_contrasts(), addressing issue #575.Problem
Previously, users could specify
es_typewith any value ofeffectsize, even though this parameter only applies wheneffectsize = "boot". This could lead to confusion as the parameter would be silently ignored for other effect size methods ("emmeans","marginal", orNULL).As noted in the issue:
Solution
Added validation that throws an informative error when
es_typeis explicitly provided buteffectsizeis not"boot":Implementation Details
The validation uses
match.call()to detect whether the user explicitly provided thees_typeparameter. This approach:es_typeChanges
estimate_contrasts.default()(10 lines)Backward Compatibility
es_typewitheffectsize = "boot"continues to workes_typecontinues to workes_typewith othereffectsizevalues will now error (this is intentional and addresses the issue)Fixes #575
Original prompt
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.