metrumresearchgroup / bbr

R interface for model and project management
https://metrumresearchgroup.github.io/bbr/
Other
22 stars 2 forks source link

param_estimates_batch breaks with newest data.table release #647

Closed barrettk closed 5 months ago

barrettk commented 5 months ago

In one of the ongoing PRs, drone failed when pulling from CRAN.

it seems to be related to how fread works here, and originates from the latest data.table release (happened yesterday).

The actual error that happens with data.table 1.15.0:

>     param_tbl <- BATCH_PARAM_TEST_DIR %>% param_estimates_batch()
Error in `rename()` at dplyr/R/mutate.R:146:3:                                                                                                                                     
! Can't rename columns that don't exist.
✖ Column `dir` doesn't exist.
Run `rlang::last_trace()` to see where the error occurred.

With data.table 1.15.0, and after running line 81 of param-estimates-batch.R (same as the link above), we end up with this data structure:

> df <- as_tibble(fread(text = res$stdout)) # line 81
> df
# A tibble: 2 × 4
  V1                  V2                   V3            V4   
  <chr>               <chr>                <chr>         <lgl>
1 dir                 error                "termination" NA   
2 test_batch_params/7 no ext file contents ""            NA  

With the previous version of data.table, we would get this:

> df <- as_tibble(fread(text = res$stdout)) # line 81
> df
# A tibble: 1 × 4
  dir                 error                termination V4   
  <chr>               <chr>                <lgl>       <lgl>
1 test_batch_params/7 no ext file contents NA          NA  

As you can see, the previous dataframe would fail when running these lines since dir is not a column name.

As an aside, Im not too sure what this is doing (It didnt change the names in my testing, though perhaps it serves a purpose in other cases?)

barrettk commented 5 months ago

FWIW res$stdout looks like this:

> res$stdout
[1] "dir,error,termination,"                     "test_batch_params/7,no ext file contents,,"

Im not aware of any other functions we could use to easily parse this into a table (without needing our own helper function), but if anyone is aware of any please let me know cc @kyleam @seth127

kyleam commented 5 months ago

@barrettk Thanks for the analysis.

Im not aware of any other functions we could use to easily parse this into a table (without needing our own helper function)

We can use fread; we just need to tell it that we know there's a header rather than letting it guess. See gh-648.