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
Bug: AUC start 0 (which also needs Feature: C0 imputation) #138
base: main
Are you sure you want to change the base?
Bug: AUC start 0 (which also needs Feature: C0 imputation) #138
Changes from all commits
0d68014
b8cd1ea
1aef135
ec3e109
50fb2e1
82521f5
135152d
18f4ee2
7e93a4f
0a5ba6d
7dbfd74
43d9f9e
429a1e6
cb1ee9c
86102f1
45e1e52
2bab21b
a622e61
c345c9b
769b80d
79d4da4
768b972
bf8844b
4411231
5c5be0a
e211bd7
4f99610
06fca2f
a6bddfc
9dfb2a6
8ad9bf1
7abe8f2
9f7fb16
0da010b
c85f76a
978c802
b9c26df
446cd63
ad4d55a
c5d834c
fda231d
baecfb8
f29cff0
54315df
de139f7
09ba650
4d608ac
91ddd4b
1485d3a
21b1b68
2a4d8b6
db2b8e8
61d3ae1
0fd56f7
cc65e0a
ed64b88
b9d7f36
3f28c24
67f7593
48d6a4c
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.
nitpick: Would suggest naming the argument
data
instead ofmydata
, or something more suggestive (pknca_data
,data_object
or similar).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.
issue: You already have
dose_group_columns
variable available.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.
question: Should this check take
drug_column
instead?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.
suggestion: Call the
mutate
function without piping the variable.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.
suggestion: Call the
mutate
function without piping the variable.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.
question:
drug_column
is already defined on line 24 and seems to be constant anyway, why re-assign it?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.
issue: From what I can see, there is no reason for collapsing a vector into a string, only to use
eval()
to turn this back into a vector? Seems to me like we can just pass those directly to a function call, likeIs there anything I am missing, why this would not work?
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.
nitpick: I would avoid hanging indents with very complex pipelines, they become hard to read pretty quickly.
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.
issue:
This is quite hard to digest and know exactly what is going on. On top of that, you have several repeating or opposite checks that can impact performance. I would suggest either:
- Breaking down the checks into separate function/s
or
- or creating columns for each check and then using that
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.
nitpick: I do not think there is a need to assign a new variable, why not just do
and remove this line.
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.
nitpick, style: no need for
return()
statement.