Failure with the new version of **blockCV**

[rvalavi]

Hi,
I've updated blockCV with a major version. Although all function names have changed to more general names/arguments, the old functions should still work fine. While reverse dependency checking I got a failure with mlr3spatiotempcv as shown below; could you please check and see if this response is expected? it seems the only issue is the numbers are switched!

You can check the new update in blockCV's repo.

`* Source code: https://github.com/cran/mlr3spatiotempcv

• Date/Publication: 2022-11-19 00:20:02 UTC
• Number of recursive dependencies: 167

Run revdepcheck::revdep_details(, "mlr3spatiotempcv") for more info

Newly broken

• checking tests ...

    Running ‘testthat.R’
   ERROR
  Running the tests in ‘tests/testthat.R’ failed.
  Last 13 lines of output:
    `expected`: 22 16
    ── Failure ('test-ResamplingSpCVBuffer.R:64'): test sets include background observations ──
    rsp$test_set(17) (`actual`) not equal to c(23, 17) (`expected`).
    
      `actual`: 17 23
    `expected`: 23 17
    ── Failure ('test-ResamplingSpCVBuffer.R:65'): test sets include background observations ──
    rsp$test_set(18) (`actual`) not equal to c(24, 18) (`expected`).
    
      `actual`: 18 24
    `expected`: 24 18
    
    [ FAIL 12 | WARN 3 | SKIP 33 | PASS 1187 ]
    Error: Test failures
    Execu`
  

[pat-s]

Thanks for the info.

Any idea why numbers change with the new update? Does the updated code apply a different sorting of the results? I think it would be easiest if the sorting of the result would stay as before to avoid having to push an update just because of this.

[rvalavi]

@pat-s yeah of course! Could you provide me with your example data so I can diagnose it?

[pat-s]

The data should not matter for this in my view.
You should be able to adapt/copy the test used in test-ResamplingSpCVBuffer.R and replicate it with your own data.

You can see what is being done starting here: https://github.com/mlr-org/mlr3spatiotempcv/blob/main/R/ResamplingSpCVBuffer.R#L119.

The values going into the test can be found here: https://github.com/mlr-org/mlr3spatiotempcv/blob/main/tests/testthat/test-ResamplingSpCVBuffer.R#L57.

[rvalavi]

@pat-s I checked and here are a few notes:
1- all blocking functions of v2.x still work (and will work for a while to allow appropriate time for adapting the new code), but a new version of them is written with new (general) names
2- there are a few new features such as hexagonal blocks for spatial cv (cv_spatial function) and the newly proposed algorithm, nearest distance distribution matching (Mila et al 2022) is added (cv_nndm); read some of the updates at https://github.com/rvalavi/blockCV#new-updates-of-the-version-30
3- I revert back function spatialBlock to exactly match the previous version to pass mlr3spatiotempcv tests; somehow the data passed to rsmp didn't work with sf::st_is_longlat for the new version of my function.
4- the values in the test (https://github.com/mlr-org/mlr3spatiotempcv/blob/main/tests/testthat/test-ResamplingSpCVBuffer.R#L57.) are the indices and their order shouldn't matter! that's why tests such as expect_equal(rsp$test_set(16), c(22, 16)) failed; I changed the order in my function for now to pass this test; please change these tests to something like: expect_equal(sort(rsp$test_set(16)), c(16, 22)) in future versions.

[pat-s]

fixed in #222