metrumresearchgroup / pmforest

Create forest plots
https://metrumresearchgroup.github.io/pmforest
Other
2 stars 0 forks source link

tests: analysis3.yml: fix BLQ value #17

Closed kyleam closed 2 years ago

kyleam commented 2 years ago

yspec 0.5.2 now errors for miscoded values. From its NEWS:

ys_load() will now detect if values / decode are mis-coded as a
list of lists; all items in values and decode must be atomic

This leads to two pmforest test failures with errors like this:

Error (test-base-plot.R:6:1): (code run outside of `test_that()`)
Error: invalid column data
In file: analysis3.yml
 column: BLQ
   - values field includes non-atomic data ...
   - yaml code possibly used brackets [ ] when braces { } were
   intended

[...]
 7. yspec:::.stop("invalid ", context, " data\n", file, "\n", err)
      at yspec/R/load_spec.R:139:2

Adjust the BLQ value to have the correct coding (see https://github.com/metrumresearchgroup/yspec/pull/108).


I've tests this with yspec 0.5.1 and 0.5.2. Both work fine with this yaml change, though in all cases I'm getting an error that looks to be unrelated to anything to do with yspec. @andersone1 is seeing a similar error on his end.

test output, with seemingly unrelated failures ``` > packageVersion("yspec") [1] ‘0.5.2’ > devtools::test() ℹ Loading pmforest ℹ Testing pmforest ✓ | F W S OK | Context ✓ | 1 20 | base-plot [6.7s] ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Skip (test-base-plot.R:284:5): Base plots: Plot alignment with different saving to different sizes [PMF-PLOT-024] Reason: SVG snapshot generated under a different vdiffr version. Please update your snapshots. ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── ✓ | 3 | input-data [0.1s] x | 1 1 | nsim-plot [1.8s] ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Failure (test-nsim-plot.R:39:5): Multiple simulations: Multiple simulations base test [PMF-PLOT-012] Snapshot of `testcase` to 'nsim-plot/multiple-simulations.svg' has changed Run `testthat::snapshot_review('nsim-plot/')` to review changes Backtrace: 1. vdiffr::expect_doppelganger("Multiple simulations", plt) at test-nsim-plot.R:39:4 3. testthat::expect_snapshot_file(...) at vdiffr/R/expect-doppelganger.R:127:2 ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── ✓ | 5 | summary [0.6s] ══ Results ════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════ Duration: 9.3 s ── Skipped tests ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── • SVG snapshot generated under a different vdiffr version. (1) [ FAIL 1 | WARN 0 | SKIP 1 | PASS 29 ] Deleting unused snapshots: • base-plot/full-test-big.svg ```
seth127 commented 2 years ago

@kyleam should we cut a 0.1.1 release to capture this on MPN? It's all test-suite stuff, so I'm not overly concerned, but we can do it pretty quickly if you think that's best.

Also, I looked into those other two failures in #18 . If we're gonna cut a new release we should probably grab those change(s) too.

kyleam commented 2 years ago

@kyleam should we cut a 0.1.1 release to capture this on MPN? It's all test-suite stuff [...]

Yeah, like you say, it's all test stuff, so I think it's fine to leave be. I'm still fighting with an unresolved failure, but I think that's about to be resolved, and I might be able to kick off a fresh build soon.