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.
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
Closes #31 wrapper functions for waist/hip and waist/height ratio #33
base: main
Are you sure you want to change the base?
Closes #31 wrapper functions for waist/hip and waist/height ratio #33
Changes from 12 commits
52edc04
91f76c3
2ce6436
be8ea17
8753a73
fddb8c3
1685761
ccdf002
eaa6524
0fd9e8f
37cfd49
84f39c4
8024de2
03f57ed
271e2a3
75c92c7
5274ef0
af058ee
283de12
65f7d89
1f05f0f
d745aca
888d2fa
d92721d
1009e76
1d649f8
058e6a5
5908433
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we might have to re-think how we handle units. For instance, if we run any of the two functions without
get_unit_expr
, the code will calculate the ratio without any regards for the actual units. I would argue that we needs to hold the hand of the user a bit here, and make sure people don't accidentally calculate a strange ratio.Here, I think we should either fail loudly, give a warning, or make units a required argument. What do you think @yurovska and @manciniedoardo ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it in the opposite way. If the unit conversion is applied, there will be an info message in the log about that.
Once
admiraldev::assert_unit()
is updated to allow checking for multiple units, I would makeget_unit_expr
a required argument in the waisthip and waisthgt functions and assert it with CDISC synonyms for length only.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Units are now asserted with CDISC synonyms for length, i.e. only
cm, m, mm, in and ft
are acceptable, so that no strange ratio is possible anymore.