nm-file: don't let custom warn value break fread warning handling

When nm_file_impl() calls data.table::read(), it catches any warning and looks at the message to decide if multiple tables were in the file, in which case it warns and returns NULL. That logic fails if the user has tweaked the warn option to, for example, silence warnings or to convert warnings to errors.

Override the warn value to 1 so that the handler can reliably process the warnings.

Note that the default warn value is 0, which says to show a message like "There were NN warnings (use warnings() to see them)" if there are over 10 messages. That setting works fine because each handler will still receive the individual warnings, but it also has no effect. So 1 is simpler value that communicates the behavior we're relying on.

This commit was originally going to be a part of a series making deprecations like the ones in gh-561 show up as errors in Drone. I was hoping I could do that with options(warn = 2), and, when you run devtools::test() with that, this test fails:

Error ('test-nm-file.R:46'): nm_file() with multiple tables warns and returns NULL [BBR-NMF-004]
Error in `fread(.path, na.strings = ".", skip = 1, verbose = FALSE)`: Stopped early on line 254. Expected 24 fields but found 21. Consider fill=TRUE and comment.char=. First discarded non-empty line: <<TABLE NO.     2: Ob\
jective Function Evaluation by Importance Sampling: Goal Function=FINAL VALUE OF OBJECTIVE FUNCTION: Problem=1 Subproblem=0 Superproblem1=0 Iteration1=0 Superproblem2=0 Iteration2=0>>
Backtrace:
  1. testthat::expect_warning(.d <- nm_file(.m, ".ext"), regexp = "does not support files with multiple tables")
       at test-nm-file.R:46:2
  7. bbr:::nm_file.bbi_model(.m, ".ext")
       at bbr/R/nm-file.R:30:2
  9. bbr:::nm_file.character(.path, .suffix = NULL)
       at bbr/R/nm-file.R:30:2
 10. bbr:::nm_file_impl(.mod)
       at bbr/R/nm-file.R:49:2
 12. data.table::fread(.path, na.strings = ".", skip = 1, verbose = FALSE)
       at bbr/R/nm-file.R:128:4

Error ('test-nm-file.R:55'): nm_file() with multiple tables swallows fread cleanup warning [BBR-NMF-004]
Error in `fread(.path, na.strings = ".", skip = 1, verbose = FALSE)`: Stopped early on line 254. Expected 24 fields but found 21. Consider fill=TRUE and comment.char=. First discarded non-empty line: <<TABLE NO.     2: Ob\
jective Function Evaluation by Importance Sampling: Goal Function=FINAL VALUE OF OBJECTIVE FUNCTION: Problem=1 Subproblem=0 Superproblem1=0 Iteration1=0 Superproblem2=0 Iteration2=0>>
Backtrace:
  1. testthat::expect_warning(.d <- nm_file(.m, ".ext"), regexp = "does not support files with multiple tables")
       at test-nm-file.R:55:2
  7. bbr:::nm_file.bbi_model(.m, ".ext")
       at bbr/R/nm-file.R:30:2
  9. bbr:::nm_file.character(.path, .suffix = NULL)
       at bbr/R/nm-file.R:30:2
 10. bbr:::nm_file_impl(.mod)
       at bbr/R/nm-file.R:49:2
 12. data.table::fread(.path, na.strings = ".", skip = 1, verbose = FALSE)
       at bbr/R/nm-file.R:128:4

Sadly, adjusting Drone is more complicated than just adding -e options(warn = 2) to the devtools::check() call, because it doesn't make it through to the underlying rcmdcheck. So, here's the fix by itself.

Note that, even though its handling doesn't trigger a test failure, check_run_times could probably use a similar fix in two spots, but I haven't taken the time to work through that code and decide whether it matters in practice.

@kyleam I think options(warn = 2) would work if we changed those tests from expect_warning() to expect_error(), right? [...]

Simply s/warning/error/? No, that wouldn't make the tests pass for at least two reasons: they'd then expect the wrong message (it'd now be the underlying one from fread rather than our custom one), and there are downstream asserts that expect an object to be returned (which wouldn't happen in the case of an error).

In any case, changing the tests so that they pass would just be hiding the core issue: nm_file_impl expects to able to swallow freads warning, and a setting of warn = 2 breaks its behavior. Below is a minimal example of the behavior that I'm referring to.

demo

``` r INPUT <- " a,b 1,2 c,d 3,4 " fn <- function() { withCallingHandlers( data.table::fread(INPUT), warning = function(x) { message("Caught warning: ", x$message) invokeRestart("muffleWarning") } ) } withr::with_options(list(warn = 1), { fn() }) #> Caught warning: Stopped early on line 4. Expected 2 fields but found 0. Consider fill=TRUE and comment.char=. First discarded non-empty line: <> #> a b #> 1: 1 2 withr::with_options(list(warn = 2), { fn() }) #> Error in data.table::fread(INPUT): Stopped early on line 4. Expected 2 fields but found 0. Consider fill=TRUE and comment.char=. First discarded non-empty line: <> ```

metrumresearchgroup / bbr

nm-file: don't let custom warn value break fread warning handling #562