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
docs: ✨ initial draft of functions to classify diabetes type #75
docs: ✨ initial draft of functions to classify diabetes type #75
Changes from 22 commits
b3d5f6d
328516b
6f7b6c2
8dc6ae1
af93e94
ef92306
18951d7
9f45a1f
332f421
b157b5a
6e9ddaa
3084d2c
175fa3d
a3655b1
f6bebfe
80fc885
12fd1d1
33d764e
6078cc7
57115bf
dadf568
f2f29fa
4295251
48c69bf
b978ce6
688fd64
b6205d9
56dda95
81b2561
80d3eab
a0f7151
616e105
1ed60d1
ff87125
7c77923
0266cee
3dc29b7
89dfe0b
cced272
8662ce4
950641b
9dacf9a
214f0e5
a6e7287
f465a63
8c45dbc
2cf719c
9c5759b
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'm not sure why we should include this in the final output. If the data coverage isn't sufficient, we shouldn't give that to users so that they make their own choice without understanding the context. We give them the context with the given assumptions and limitations of this algorithm.
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.
@Aastedet ? :)
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.
@lwjohnst86 At the Steering Group meeting we decided to provide both, but the "stable_inclusion" being the clear default, while we are explicit to the user that the "raw_inclusion" is experimental/use-at-own-risk.
I'm not sure if that guides us towards an answer here though. Depending on the study design, the user might have a clear need for the "raw_inclusion" date.
I lean towards including both, but naming them to "inclusion_date" and something like "unstable_date" or "_date" to make it clear which is the default (and also obscuring the non-default variable name so users will have to read the documentation to know what the variable is).
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.
Hmmm, what about
initial_inclusion_date
andinclusion_date
? Just wondering how easily a user might interpret "unstable".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'm not sure about "initial". It sounds like it's something that's before something else? Like e.g., the first inclusion event.
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.
Maybe
inclusion_date
andinclusion_date_with_insufficient_data
?