revdep CornerstoneR affected

[mattdowle]

Same as #5125.

It does this on line 232 of fitFunction.R :

names(dtStart)[-seq_along(groups)] = par.names

which now drops dtStart's key and results later in the errors below.
It's harder to see how this one can be converted to setnames() so I'm leaning towards finding a way to retain the key, rather than contact the revdep maintainer.

> test_check("CornerstoneR")
══ Failed tests ════════════════════════════════════════════════════════════════
── Failure (test_fitFunction.R:95:3): Work with Invalid Variable Names ─────────
Check on res$coeff is not TRUE

`actual` is a character vector ('Contains missing values (column \'Coeff_a\', row 1)')
`expected` is a logical vector (TRUE)
Contains missing values (column 'Coeff_a', row 1)
Backtrace:
    █
 1. └─checkmate::expect_data_table(...) test_fitFunction.R:95:2
 2.   └─checkmate::makeExpectation(x, res, info, label)
 3.     └─testthat::expect_true(...)

which is because of these errors try()-ed and caught and places within dtCoeff due to missing key :

$coeff
   X1.grp Coeff_a Coeff_b Coeff_c Coeff_d StdErr_a StdErr_b StdErr_c StdErr_d pseudoR2  RMSE Converged Iterations Tolerance StopCode
   <char>   <num>   <num>   <num>   <num>    <num>    <num>    <num>    <num>    <num> <num>    <lgcl>      <num>     <num>    <num>
1:      A      NA      NA      NA      NA       NA       NA       NA       NA       NA    NA        NA         NA        NA       NA
2:      B      NA      NA      NA      NA       NA       NA       NA       NA       NA    NA        NA         NA        NA       NA
                                                                                                                                                                                                                                                                                                                                                                 StopMessage
                                                                                                                                                                                                                                                                                                                                                                      <char>
1: Error : When i is a data.table (or character vector), the columns to join by must be specified using 'on=' argument (see ?data.table), by keying x (i.e. sorted, and, marked as sorted, see ?setkey), or by sharing column names between x and i (i.e., a natural join). Keyed joins might have further speed benefits on very large data due to x being sorted in RAM.\n
2: Error : When i is a data.table (or character vector), the columns to join by must be specified using 'on=' argument (see ?data.table), by keying x (i.e. sorted, and, marked as sorted, see ?setkey), or by sharing column names between x and i (i.e., a natural join). Keyed joins might have further speed benefits on very large data due to x being sorted in RAM.\n

[tdhock]

git bisect says that this error started happening after merging #5084

[mattdowle]

Thanks for doing the bisect, but did you see #5133 linked above which mentions that?

[MichaelChirico]

FWIW, I don't reproduce this error on the CRAN version of {CornerstoneR} or on their GitLab current master, using the master version of {data.table}.

I do, however, see the guilty usage still in their sources -- I'm just not sure if (1) they fixed something elsewhere in the repo (good!) or (2) they just deactivated some tests (not great).

https://gitlab.com/camLine/CornerstoneR/-/blob/master/R/fitFunction.R?ref_type=heads#L297

[tdhock]

I believe we should keep this open until the revdep fixes this, or until it disappears from the revdep checker (currently still there) https://rcdata.nau.edu/genomic-ml/data.table-revdeps/analyze/2023-12-22/

[MichaelChirico]

Hi @surmann, I'd like to send a PR fixing this issue in {CornerstoneR}, but the GitLab source does not accept forks. Please advise how to proceed.

[surmann]

Hallo @MichaelChirico
I'm not responsible for this repo any more. Please contant Gerrith.
All the best
Dirk

[MichaelChirico]

Now fixed+verified: