sort ids via vctrs, pass sorting to pull_*() helpers
#730
Conversation
| resamples <- pull_notes(resamples, results, control) | ||
| resamples <- pull_extracts(resamples, results, control) | ||
| resamples <- pull_predictions(resamples, results, control) | ||
| # carry out arranging by id before extracting each element of results (#728) |
There was a problem hiding this comment.
Previously this sorting happened independently for each of the pull_*() functions that call pulley(). For those that don't call pulley(), the ordering is wrong.
| resamples <- pull_extracts(resamples, results, control) | ||
| resamples <- pull_predictions(resamples, results, control) | ||
| # carry out arranging by id before extracting each element of results (#728) | ||
| resample_ids <- grep("^id", names(resamples), value = TRUE) |
There was a problem hiding this comment.
This is the pattern used by pulley().
| resamples <- pull_predictions(resamples, results, control) | ||
| # carry out arranging by id before extracting each element of results (#728) | ||
| resample_ids <- grep("^id", names(resamples), value = TRUE) | ||
| id_order <- vctrs::vec_order(resamples[resample_ids]) |
There was a problem hiding this comment.
I don't suspect this is any different than passing those columns to arrange() (some locale technicalities maybe?), but worth looking into.
There was a problem hiding this comment.
arrange() uses vec_order_radix() under the hood. but I don't think it should matter much here
| @@ -65,22 +71,22 @@ maybe_repair <- function(x) { | |||
| } | |||
|
|
|||
|
|
|||
| pull_metrics <- function(resamples, res, control) { | |||
There was a problem hiding this comment.
The remainder of the changes in this file just newly pass order along to pulley().
| resamples <- pull_metrics(resamples, results, control, order = id_order) | ||
| resamples <- pull_notes(resamples, results, control, order = id_order) | ||
| resamples <- pull_extracts(resamples, results, control, order = id_order) | ||
| resamples <- pull_predictions(resamples, results, control, order = id_order) | ||
| resamples <- pull_all_outcome_names(resamples, results) |
There was a problem hiding this comment.
Note that this function doesn't gain an order argument, as its output doesn't end up in tuning results.
simonpcouch
requested review from
topepo and
EmilHvitfeldt
and removed request for
topepo
|
This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue. |
Closes #728.