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

Add prop_diff_stratnc() calculating the stratified Newcombe CI #588

Closed
4 tasks done
bahatsky opened this issue Jun 9, 2022 · 7 comments · Fixed by #696
Closed
4 tasks done

Add prop_diff_stratnc() calculating the stratified Newcombe CI #588

bahatsky opened this issue Jun 9, 2022 · 7 comments · Fixed by #696
Assignees

Comments

@bahatsky
Copy link
Contributor

bahatsky commented Jun 9, 2022

For a study we are working on a shiny app and one of the outputs should look like below. To enable this we would need to have stratified Newcombe confidence interval (as there are strata).

see https://documentation.sas.com/doc/en/pgmsascdc/9.4_3.5/procstat/procstat_freq_details63.htm and references therein for the method

To do:

  • design
  • implementation
  • examples
  • tests
@shajoezhu shajoezhu added the sme label Jul 19, 2022
@shajoezhu
Copy link
Contributor

From @bahatsky ,

https://blogs.sas.com/content/iml/2022/04/18/mcnemar-test-sas.html could be a good starting point.

@danielinteractive danielinteractive self-assigned this Sep 1, 2022
@danielinteractive
Copy link
Contributor

@shajoezhu I am doing this now since we need it for the Crova dry run

@danielinteractive danielinteractive changed the title Add p-Value (McNemarTest) to test_proportion_diff Add p-Value (stratified Newcombe) to test_proportion_diff Sep 1, 2022
@danielinteractive danielinteractive changed the title Add p-Value (stratified Newcombe) to test_proportion_diff Add strata option to prop_diff_nc() to obtain stratified Newcombe CI Sep 1, 2022
@danielinteractive danielinteractive changed the title Add strata option to prop_diff_nc() to obtain stratified Newcombe CI Add prop_diff_stratnc() calculating the stratified Newcombe CI Sep 1, 2022
@shajoezhu
Copy link
Contributor

Thanks so much @danielinteractive

@danielinteractive danielinteractive removed their assignment Sep 5, 2022
@Melkiades Melkiades self-assigned this Sep 5, 2022
@Melkiades
Copy link
Contributor

Melkiades commented Sep 6, 2022

I am adding also prop_strat_wilson (with and without continuity correction) and Yan and Su (2010) heuristic which is used to estimate weights. In Daniel's design, the stratified Newcombe is calculated with weights that come from a modified CMH which now outputs the found weights. As I implemented now the heuristic, it outputs too the found weights (I might add the switcher for that in stratnc later. Here I add some points for me to keep track of the latest developments:

  • Add directly tests for estimate_proportion when used with rtables (not related to current design, it was missing)
  • Implement strat_wilson from Daniel's design
  • Add heuristic for weights
  • Documentation
  • Examples
  • Tests
  • Adapt s_proportion to work with strata
  • Adapt estimate_proportion to work with df and therefore strata for this specific case
  • Add more tests and examples for the latter specific case (behavior with rtables)

@danielinteractive
Copy link
Contributor

thanks @Melkiades for working on this!

@Melkiades
Copy link
Contributor

Melkiades commented Sep 6, 2022

Thanks to you for the great initial design! I just completed the first part (s_proportion and prop_strat_wilson). At the beginning, I took out the suppressWarnings() around stats::prop.test(), but when I added the real data test is complaining a lot about an approximation done. Do you know what is this behavior precisely?

I have already asked for your review of the code, and I really tried my best to make s_proportion fit the new structure but I am sure there is space for improvement there as I took inspiration from prop_diff directly. Any suggestion is super welcome!

Note: it is still missing the stratified Newcombe. I will work on that tomorrow ;)

@danielinteractive
Copy link
Contributor

Thanks @Melkiades - yeah we should suppress those warnings as we are not interested in them here. Due to the stratification there is small numbers everywhere, but the method is as it is, so it is supposed to use the small numbers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants