RXR-372: refactor calc_ibw() #12
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Not sure we'll do this, but it'll be easy enough
karawoo
commented
Jul 15, 2021
Comment on lines
+73
to
+76
| ibw <- switch( | ||
| method_children, | ||
| "standard" = ibw_standard(age = age, height = height, sex = sex) | ||
| ) |
There was a problem hiding this comment.
The documentation lists some other methods for children that might be implemented in the future, so I put switch() here to make it easy to add those, even though for now we only have one method.
jasmineirx
approved these changes
Jul 16, 2021
| if (is.null(weight)) { | ||
| stop("Actual body weight is used as IBW for children < 1yr. Please supply a weight value.") | ||
| } | ||
| message("Note: using actual body weight as IBW for children < 1yr.") | ||
| ibw[age < 1] <- weight[age < 1] | ||
| return(weight) |
There was a problem hiding this comment.
it's a little odd that ibw is only rounded to digits digits if age >= 1. IMO rounding should happen outside of this function and not within it. However, that's better addressed in a separate change.
made an issue (RXR-405) to track.
There was a problem hiding this comment.
Ah yeah good point. I agree rounding should happen outside the function 👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
calc_ibw()had a number of bugs and inconsistencies in its behavior. This PR refactors it into a smaller, less complicated function with a couple of helper functions.The main difference in behavior is that
calc_ibw()no longer provides a vectorized interface. The previous vectorized operations were pretty buggy, and it's really very different data and operations that are getting used depending on the age that's provided to the function. I think this implementation is a lot cleaner and easier to maintain.I looked at our downstream usage in our internal packages, and from what I can tell it seems we're not using the vectorized interface anyway, so I think this change shouldn't impact those packages. That said, I'm open to trying to bring back support for vector input if we think it's important (though the behavior should probably be different than before; see my comments on the RXR-372 jira ticket).