Closed JSchoenbachler closed 2 years ago
Fixed some minor spacing and formatting issues, beginning code review.
.packages
in foreach
might not be necessary, never used before.foreach
functions and fixes for the data.table
column name issues with running R CMD check
.weightsAll
and the subsequent merge seem like there might be some unnecessary (or rather inefficient) processing being done. For instance, if we are setting dx_status
as 0 if is.na(phecode)
and the subsequent weight calculation is therefore 0, why are we using all.x = TRUE
in the merge for weightsSub
? I think this could be revisited.weightsSub
could probably be a function of it's own. I know it isn't re-used anywhere currently, but it might just be useful to have it broken up. I'm undecided on this though.calcWeights
, I think you need to specify data.table::uniqueN
since you're not importing the whole package.calcPheRS
takes GRIDs and phecodes, it seems like it should just call calcWeights
on its own instead of the weights having to be provided separately. Maybe if the weights argument is missing it could do that?I'm marking this first code review as "Done" on the project, but keeping the Issue set as "Open" for ease of access while work is being done on the package.
Elliot and I to review Layla's first commit individually.