Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI docs about 1-sided tests + default 90% CI for phi/V #366

Merged
merged 21 commits into from
Aug 18, 2021
Merged

Conversation

mattansb
Copy link
Member

@mattansb mattansb commented Aug 16, 2021

Close #365

@IndrajeetPatil Can you see if I missed any 0.95 to 0.9 changes? No longer relevant. But you can take a look anyway - you always catch somethig.

@bwiernik Can you look at help("effectsize-CIs", package = "effectsize") - the new Choosing the Appropriate CI Level section, see if it is clear / mistake free?


TODO:

  • extra docs - change "which level" to explain on sided CI
  • vignette
  • effectsize
    • effectsize.htest
    • effectsize.BF?
  • eta_squared
    • eta_squared_posterior?
  • rank
  • std_params One-sided CIs parameters#584
  • equi tests!
  • TESTS:
    • Printing
    • 90-2 vs 95-1
    • htests

@bwiernik
Copy link
Contributor

bwiernik commented Aug 16, 2021

I'm still not sure I like this approach versus using 1-tailed CIs. If we want to ensure the CI and test decision rules are consistent, I think the safest way to do that is to make the construction consistent by matching the number of tails.

I think it's as confusing to have a different alpha rate than usual as default and in the table header as it is to potentially run into a situation where 1-tailed p < .05 but 2-tailed 95% CI includes 0. I suspect a fair number of users change it to .95, potentially opening that door for confusion.

Or, if they leave it .90, they run the risk of reviewer pushback like you got. I can imagine this being a source of "QRP questionable research practice" accusations by a reviewer if they didn't understand the nuance here. This is probably the most problematic situation open here and many users won't be prepared to respond effectively in that situation.

I don't really see much downside to reporting 1-tailed CIs. I agree that [.04, Inf] or [.04, 1.0] would be confusing, but if we reported it as > .04 or Lower: .04 or another very different format, I think both be strictly correct and clearer that it's intentional. That could lead users to choosing consulting documentation and making an informed choice about a 2-tailed interval for these cases if they so choose.

Looking at it, I kind of like 95% CI > .04—that is strictly accurate and not ambiguous.

@bwiernik
Copy link
Contributor

bwiernik commented Aug 16, 2021

Something like this perhaps:

(etas <- F_to_eta2(
    f = c(40.72, 33.77, 45.31),
    df = c(2, 1, 2),
    df_error = c(18, 9, 18),
    ci = .95, tails = "lower"
))
# Eta2 (partial) |      95% CI
# ----------------------------
# 0.82           |  [ > 0.66 ]
# 0.79           |  [ > 0.49 ]
# 0.83           |  [ > 0.69 ]

@mattansb
Copy link
Member Author

You're starting to convince me....

I'm thinking of having the argument alternative = c("two.sided", "less", "greater") and the output [0.6, Inf], to align with what what the *test() functions give.

Let me think this over...

@bwiernik
Copy link
Contributor

alternative matching arguments with *test() would be good.

As for [0.6, Inf] versus [> 0.6], my main question would be whether a user would think that Inf was a computational failure or not. [> 0.6] might be clearer that this is an okay result.

I think keeping Inf or -Inf in the data frame makes sense, but maybe format_ci() should change c(.06, Inf) to something else for display?

@mattansb
Copy link
Member Author

I think a good footnote informing the one sided CI will solve any confusion.

@bwiernik
Copy link
Contributor

perfect.

@codecov-commenter
Copy link

codecov-commenter commented Aug 16, 2021

Codecov Report

Merging #366 (1305743) into main (bd20d6c) will increase coverage by 0.10%.
The diff coverage is 77.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #366      +/-   ##
==========================================
+ Coverage   80.78%   80.88%   +0.10%     
==========================================
  Files          48       48              
  Lines        2971     3113     +142     
==========================================
+ Hits         2400     2518     +118     
- Misses        571      595      +24     
Impacted Files Coverage Δ
R/convert_between_OR_to_RR.R 100.00% <ø> (ø)
R/convert_between_d_to_common_language.R 100.00% <ø> (ø)
R/convert_between_d_to_r.R 85.71% <ø> (ø)
R/convert_between_odds_to_probs.R 79.48% <ø> (ø)
R/effectsize.R 75.00% <ø> (ø)
R/effectsize.htest.R 67.07% <61.53%> (-0.48%) ⬇️
R/equivalence_test.R 80.55% <68.18%> (+12.98%) ⬆️
R/convert_stat_to_d.R 90.32% <68.42%> (ø)
R/xtab.R 78.67% <73.33%> (-0.78%) ⬇️
R/convert_stat_to_r.R 90.00% <73.68%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd20d6c...1305743. Read the comment docs.

@mattansb
Copy link
Member Author

What do you think?

library(effectsize)
cohens_d(mpg ~ am, data = mtcars, ci = 0.9)
#> Cohen's d |         90% CI
#> --------------------------
#> -1.48     | [-2.14, -0.80]
#> 
#> - Estimated using pooled SD.
cohens_d(mpg ~ am, data = mtcars, ci = 0.95, alternative = "g")
#> Cohen's d |            95% CI
#> -----------------------------
#> -1.48     | [-2.14,      Inf]
#> 
#> - Estimated using pooled SD.
#> - One-sided CIs: upper bound fixed at (Inf).
cohens_d(mpg ~ am, data = mtcars, ci = 0.95, alternative = "l")
#> Cohen's d |        95% CI
#> -------------------------
#> -1.48     | [-Inf, -0.80]
#> 
#> - Estimated using pooled SD.
#> - One-sided CIs: lower bound fixed at (-Inf).

Created on 2021-08-16 by the reprex package (v2.0.1)

@mattansb mattansb self-assigned this Aug 16, 2021
@bwiernik
Copy link
Contributor

bwiernik commented Aug 16, 2021

Looks good. Something looks odd on the printed width of the positive Inf CI formatting

@mattansb
Copy link
Member Author

@bwiernik Can you take a look at the new help("effectsize-CIs", package = "effectsize")? I'm not sure what I wrote isn't too convoluted...

@bwiernik
Copy link
Contributor

I'll make some revisions.

@mattansb
Copy link
Member Author

@strengejacke does parameters need any updating to accommodate this change to CIs?

@DominiqueMakowski @IndrajeetPatil for modelbased or correlation?

report would have to mention the one sided CIs somehow.

@IndrajeetPatil
Copy link
Member

Thanks for checking with me.

I am not going to have access to my computer again until the 30th of August, so can't really do much.
But I know that this is going to lead to tons of failing tests in statsExpressions for sure, so please hold off on CRAN submission until you hear from me. 🙇‍♂️

NEWS.md Outdated Show resolved Hide resolved
@bwiernik
Copy link
Contributor

bwiernik commented Aug 17, 2021

@mattansb I revised the new section of the help("effectsize_CIs", "effectsize") document to be a more straightforward. I also streamlined some grammar in the other sections and made some revisions to be more accurate (e.g., when p < alpha/2 in an F distribution, the bounds being set to 0 per Steiger isn't really arbitrary).

Let me know what you think.

#' @inheritSection effectsize-CIs CI Contains Zero
#' @inheritSection effectsize_CIs Confidence (Compatibility) Intervals (CIs)
#' @inheritSection effectsize_CIs CIs and Significance Tests
#' @inheritSection effectsize_CIs One-Sided CIs
Copy link
Member Author

Choose a reason for hiding this comment

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

@bwiernik I didn't originally add these as this seemed to much make the individual docs way too long for my liking. We can reference this and the troubleshooting in the same line.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Fine by me.

@mattansb
Copy link
Member Author

@strengejacke I'm leaving standardized_parameters as-is - any supported solution would actually need to be in `{parameters} first.

@mattansb
Copy link
Member Author

@IndrajeetPatil

But I know that this is going to lead to tons of failing tests in statsExpressions for sure, so please hold off on CRAN submission until you hear from me. 🙇‍♂️

I'm not sure when effectsize is schedualed to be updated. AFAIK it's not in the pipes, right?
If there should be one soon, I can hold off on the merge of this PR. If not, I'll merge... now.

@IndrajeetPatil
Copy link
Member

You can merge now, but can wait to push it to CRAN till I am back.

Even if it was not scheduled to be updated for this cycle, I would vote for doing so given the significant number of changes that have happened since the last release. It will be good to keep the whole ecosystem in sync when we release {easystats} meta-package on CRAN.

@strengejacke
Copy link
Member

@strengejacke does parameters need any updating to accommodate this change to CIs?

Let me check...

@mattansb mattansb merged commit 8f7b524 into main Aug 18, 2021
@mattansb mattansb deleted the more_ci_docs branch August 18, 2021 13:09
@strengejacke
Copy link
Member

I'm not sure where this would apply... Anova?

@IndrajeetPatil
Copy link
Member

@mattansb You no longer need GitHub version of {datawizard}, since it’s on CRAN.

@mattansb
Copy link
Member Author

I'm not sure where this would apply... Anova?

ANOVA and some of the htests.

@strengejacke
Copy link
Member

type: as as such are generally tested using 2-tailed tests and 2-sided CIs.

@mattansb
Copy link
Member Author

Is that a question? I can't parse it ^_^

@strengejacke
Copy link
Member

ups, a typo. "type" should be "typo" :-D

@strengejacke
Copy link
Member

What I wanted to say that there's a typo in the effectsize-CIs docs

@mattansb
Copy link
Member Author

lol

@bwiernik
Copy link
Contributor

"as as such are generally" should be "and as such are generally" in the Docs

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.

default to 95% confidence interval for effect size conversion functions?
5 participants