error with LEMON on `is.integer(node_info[['price']])`

[josherrickson]

We're seeing failures in the test suite when using rlemon:

Error (test.fmatch.R:147): Passing and receiving node information
Error in `fmatch(pm, 2, 2, node_info = n0, solver = slvr)`: is.integer(node_info[["price"]]) is not TRUE
Backtrace:
  1. testthat::expect_silent(fmatch(pm, 2, 2, node_info = n0, solver = slvr))
       at test.fmatch.R:147:2
  9. optmatch:::fmatch(pm, 2, 2, node_info = n0, solver = slvr)
 10. base::stopifnot(...)
       at optmatch/R/fmatch.R:40:2

I believe this is due to rlemon returning numeric, not integer:

Browse[2]> node_info[['price']]
[1] 1 1 1 0 0 0 0 0
Browse[2]> is.integer(node_info[['price']])
[1] FALSE
Browse[2]> is.numeric(node_info[['price']])
[1] TRUE

Options:

1. We can change the is.integer to is.numeric.
2. is.integer to is.numeric, and then ensure whole numbers (all.equal(round(x), x) or something similar?)
3. We can condition on LEMON vs RELAX; checking integer for RELAX and numeric for LEMON.
4. We can look into rlemon and see whether it should be returning an integer.

This is high priority - we have until 2/12 to get a new version pushed up. @benthestatistician

[josherrickson]

Just a note that we're not catching this locally because the tests where they're failing check if rrelaxiv is available and use it if so, whereas CRAN will not have access to it. Removing rrelaxiv locally and running the test suite fixes it.

[josherrickson]

Just pushed up an adjustment to the offending files that will ensure both LEMON and RELAX (if installed) get tested.

[josherrickson]

In order to hit our deadline, I'm going with 3 above - condition on LEMON vs RELAX; checking integer for RELAX and numeric for LEMON.

I'll be pushing up a new version shortly. If we want to revisit this decision, we can do so for the next release.

[benthestatistician]

I agree with this solution, both for now and moving forward. (RELAX-IV requires integer data, so fmatch() should check that that's satisfied when calling it; but otherwise it's not an intrinsic requirement, so if other solvers take non-integer numerics then we should let 'im.) Thanks J! (I'll leave it to you to close out the issue.)