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 general corrections, add fixes to be able to compile the book #10

Open
wants to merge 11 commits into
base: corrections
Choose a base branch
from

Conversation

vkhodygo
Copy link

@vkhodygo vkhodygo commented Nov 3, 2023

This should close #2, partially #3 (was is just rebuilding that fixed the typesetting issue?), & #9.

I had to employ some dirty fixes to make it work due to missing datasets. This has been raised in #1 already, and it is understandable you might not be able to share all of the data due to it being personal. I'd suggest using synthetic datasets (I could give you a hand with this one!) or find a replacement. It's difficult to reproduce any results without the dataset itself. But I digress. One of the remaining issues is that the following bit fails

fimdbook/R/fimd.R

Line 2451 in 6627496

as.vector(exp(summary(pool(fit))[, 1]))

In my case I had to replace it with

as.vector(exp(as.numeric(summary(pool(fit))[, 1])))

However, I'm not sure if it's the code itself or my borked data that causes this. Feel free to add the change if you know what's right.

Also, there is some redundant code left, and there is little context to choose one of the lines:

fimdbook/R/fimd.R

Lines 2375 to 2376 in 6627496

mis <- ici(data2[, vnames])
mis <- is.na(imp$data$rrsyst) | is.na(imp$data$rrdiast)

Finally, I'd change the default branch to this one to let people know explicitly the latest corrected version available.

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.

1 participant