-
Notifications
You must be signed in to change notification settings - Fork 12
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
use hardhat::quantile_pred #332
base: main
Are you sure you want to change the base?
Conversation
💢
|
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.
Looks good!
possible_cols <- c(".pred_quantile", ".pred_lower", ".pred_upper") | ||
existing_cols <- intersect(possible_cols, nms) |
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.
nice naming, it's easy to read!
} | ||
|
||
re_nest <- function(df) { | ||
.row <- 1:nrow(df) |
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.
.row <- 1:nrow(df) | |
.row <- seq_len(nrow(df)) |
expect_equal( | ||
tidyr::unnest(pred, cols = .pred)$.pred_quantile, | ||
do.call(rbind, exp_pred)$est | ||
) |
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.
Can we keep the check on the values here, like we did for the flexsurv
engine? I don't mind if the checks on the confidence intervals below only check the structure.
# column with co.lumns ".quantile" and ".pred_quantile" and perhaps | ||
# ".pred_lower" and ".pred_upper" | ||
|
||
flexsurv_to_quantile_pred <- function(x, object) { |
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.
There is already another function to use in the post
hook of the prediction module, called flexsurv_post()
. Can we call this one flexsurv_post_quantile()
?
df | ||
} | ||
|
||
nested_df_iter <- function(df, col) { |
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.
How about col_to_quantile_pred()
as a name?
@@ -57,7 +57,7 @@ jobs: | |||
extra-packages: | |||
any::rcmdcheck, | |||
aorsf=?ignore-before-r=4.2.0, | |||
pec=?ignore-before-r=4.3.0 | |||
pec=?ignore-before-r=4.4.0 |
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.
👀
Changes for tidymodels/parsnip#1203 to convert quantile predictions to the new vctrs class.