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

Test statistic not showing any longer #48

Closed
kylerove opened this issue Apr 19, 2019 · 11 comments
Closed

Test statistic not showing any longer #48

kylerove opened this issue Apr 19, 2019 · 11 comments
Labels

Comments

@kylerove
Copy link

I don't know when this changed, but its been a few months since I ran my code. Now it is not showing the test statistic column. I tried with no transform, hmisc, my own transform. What am I missing?

table1 <- tangram(group ~ age[2] + sex + height[1] + weight[1] + bmi[4] + bmi_z[2] + tmi[6] + race + language + insurance + distance[1] + match_vp_shunt + match_spina_dx + match_past_surgery + neurologic_lesion + neurologic_lesion_level + match_ambulation + diagnosis_1 , data=recordsData, transform=hmisc, id="table1")

@spgarbet
Copy link
Owner

There has been some debate and back and forth about the test statistic present / absence. Add test=TRUE as an argument and I think that will work.

Other things, the id is now pulled from the R chuck id if possible if you're using Rmarkdown. Also if you just call it in a chuck (i.e. drop assignment and later render call) it will autodetect if it's being called from Rmarkdown and find the right renderer to use, so a document will compile to either PDF or HTML with no code changes.

I've also added documentation for the transforms, for example ?hmisc will show you the arguments that get passed in.

@kylerove
Copy link
Author

Sweet thank you! I tend to write my analyses and output independent results (tables, figures). Just haven't found a good resource to get into rmarkdown workflow. Would love to integrate with versioning/git. Again, this package is awesome.

I will have to write my own function for test statistics....have a project that uses propensity matching. Will want matching-appropriate comparisons. All the best!

@spgarbet
Copy link
Owner

I want to add more transforms. I will write the guts of it if you provide me an Rmd file showing the table layout you desire, the types to compare. I do have one currently that was contributed smd that is in the package that allows for propensity weighting. Might want to check that transform out. Anyway, send me something that generates test data, some example of the test and a sketch and I can turn around a repeatable transform for you and integrate it into the package. Remember to define all comparisons desired that are sensible (e.g. Numeric X Cat, Cat X Numeric, Numeric X Numeric, Cat X Cat) and I'll list you as a co-author.

@kylerove
Copy link
Author

Boom. Finally got around to it. Here is Rmd: https://github.com/kylerove/tangram_psm

Everything appears to work well and runs without errors, but the tables won't display because:

Error in table_flatten(x) : 
  (list) object cannot be coerced to type 'logical'
In addition: Warning message:
In any(sapply(table, function(y) sapply(y, function(z) inherits(z,  :
  coercing argument of type 'list' to logical

This probably has something to do with the table-building code. Anyway, let me know what you think. I hope we can get this working soon!

@spgarbet
Copy link
Owner

That warning is interesting. It's saying the the type detection is failing and it's trying to coerce a list to logical. Should be a minor tweak.

@spgarbet
Copy link
Owner

This is odd, because sex and lang work individually and technically should have no interaction in the code as it pastes them together after running separately. This means that some global variable is leaking between the two.

> tangram(group ~ sex, data=m1.final, pref_test = "wilcox.test", block=m1.final$block, test=TRUE, transform = psm)
===================================================
     N        0             1        Test Statistic
            (N=30)        (N=30)                   
---------------------------------------------------
sex  60  22 (73.333%)  22 (73.333%)        —       
===================================================
Numerical summary is median (IQR). Categorical is N (%). ^1 Logistic regression with GEE. ^2 Cochran Mantel Maenszel.Warning message:
In tangram.formula(group ~ sex, data = m1.final, pref_test = "wilcox.test",  :
  tangram() will require unique id to be specified in the future
> tangram(group ~ lang, data=m1.final, pref_test = "wilcox.test", block=m1.final$block, test=TRUE, transform = psm)
======================================================
      N        0             1        Test Statistic  
------------------------------------------------------
             (N=30)        (N=30)                     
lang  60                                  0.630       
   1      12 (40.000%)  8 (26.667%)                   
   2      6 (20.000%)   7 (23.333%)                   
   3      6 (20.000%)   10 (33.333%)                  
   4      6 (20.000%)   5 (16.667%)                   
======================================================
Numerical summary is median (IQR). Categorical is N (%). ^1 Logistic regression with GEE. ^2 Cochran Mantel Maenszel.Warning message:
In tangram.formula(group ~ lang, data = m1.final, pref_test = "wilcox.test",  :
  tangram() will require unique id to be specified in the future
> tangram(group ~ sex+lang, data=m1.final, pref_test = "wilcox.test", block=m1.final$block, test=TRUE, transform = psm)
Error in table_flatten(x) : 
  (list) object cannot be coerced to type 'logical'
In addition: Warning messages:
1: In tangram.formula(group ~ sex + lang, data = m1.final, pref_test = "wilcox.test",  :
  tangram() will require unique id to be specified in the future
2: In any(sapply(table, function(y) sapply(y, function(z) inherits(z,  :
  coercing argument of type 'list' to logical

@kylerove
Copy link
Author

Yeah, I'm not super adept at debugging tools in R. I usually resort to print() statements to narrow down lines that might be generating the issue. This one though, stumped me because no actual error comes out until you try to display the result.

@spgarbet
Copy link
Owner

spgarbet commented Sep 20, 2019

Okay, there's a lot to unpack on this one. First the fix is easy.

Change line 424 in Matched cohort stats from add_col to add_row.

So the error message you got was because the sub-tables didn't line up. I had the same problem and fixed it in the original you copied from.

I see several things that should happen because of this:

  1. The error message needs to be descriptive to triage the problem faster. This was a really simple problem once it was obvious what it was. However logical conversion issues are nonsensical. So I'm going to add much better error handling that checks for mis-specified transforms.
  2. There is a huge amount of boiler plate code copied over to meet your end goal. It would be far better that only the necessary overrides were required, making this use case much simpler to implement. However, kudos for diving into the deep end. I will work on turning this into a much simpler method that makes the same tables.
  3. I'd like to include this use-case as another available transform for users. Do I have your permission to use this code and include you as a contributing author?

@kylerove
Copy link
Author

kylerove commented Sep 20, 2019 via email

@kylerove
Copy link
Author

kylerove commented Sep 23, 2019

I made an error in my mapping of the various comparison contingencies (numerical x categorical, etc). I didn't realize it. Because the only valid options for comparison should be numerical x cat and cat x cat, I have removed the other contingency options within psm() in my .rmd example.

The function summarize_numerical() is still generating the table improperly and generating an error. I've played around with it for a few hours, but can't find the issue which is frustrating. :/

Anyway, look forward to integrating this transform into your package.

spgarbet added a commit that referenced this issue Sep 23, 2019
@spgarbet spgarbet added the bug label Sep 23, 2019
@spgarbet
Copy link
Owner

I found a simple fix to prevent this issue from being a problem. Going to close this bug ticket and open an enhancement for the other issues.

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

No branches or pull requests

2 participants