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
issue with pre functions #491
issue with pre functions #491
Changes from 5 commits
e0f2a37
80475b9
034410a
29046c7
4748308
a50d239
e3b6aba
df9642d
2c6a821
44aa092
811ce71
94d0c0e
8c49c6e
6d13c28
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
Not sure if missing should appear (I dont think so). What do you think @barnett11
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.
this is a difficult issue. We can expect
Missing
grade in adae.atoxgr, (even in cleaned adam). We should not filter this out because we also need to count the number of events. But this should not be counted in thecount_occurrences_by_grade
.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.
ok. for
ATOXGR
, those records are still usingNA
(not convertet to a level) and then will be discarded.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.
another update: ADAE.ATOXGR is a string (
character
in R's language). in rtables we require ATOXGR to befactor
. Then we need to convert it into factor.so here while we apply reformating, we need to populate
NA
for empty strings. Very different logic here!my suggestion is to keep this missing level for now (as this solves many issues) and open another issue to deal with this.
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.
probably need to evaluate the
reformat
when it is partial match of the whole set, option 1, keep the rest, option 2, discard the rest levels (use NA to replace) (but this is really contradictory to what we are doing to conver NA to "Missing"!) The logic/design behind this need to be further evaluated.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 opened here: #492