Refactor/cli messages by timcadman · Pull Request #731 · datashield/dsBaseClient

timcadman · 2026-11-05T10:47:54Z

Backgound

Previously error messages were a bit hit and miss in DataSHIELD, and not always to understand exactly what went wrong.

What's changed

Use CLI package for more attractive message formatting
Use tidyverse style guide (https://style.tidyverse.org/errors.html) for structure on writing error messages
Expectations of many tests updated to capture the new error messages

How to test

Use the script below. This will print a comprehensive set of errors so you can manually review that they are clear.

library(DSI)
library(DSLite)
library(remotes)

dslite_lib <- tools::R_user_dir("dsBaseClient-demo", "data")
dir.create(dslite_lib, recursive = TRUE, showWarnings = FALSE)
.libPaths(c(dslite_lib, .libPaths()))
remotes::install_github("datashield/dsBase", ref = "v7.0-dev", lib = dslite_lib, upgrade = "never")
devtools::load_all()

options(datashield.errors.print = TRUE)

login.data <- DSLite::setupDATASETTest("dsBase")
dslite.server$option("datashield.privacyControlLevel", "'permissive'")
conns <- datashield.login(logins = login.data, assign = TRUE)

ds.asCharacter(x.name = "D$NUMERIC", newobj = "mixed", datasources = conns[1])
ds.asNumeric(x.name = "D$NUMERIC", newobj = "mixed", datasources = conns[2:3])
ds.asInteger(x.name = "D$NUMERIC", newobj = "mixednum", datasources = conns[1])
ds.asNumeric(x.name = "D$NUMERIC", newobj = "mixednum", datasources = conns[2:3])
ds.rep(x1 = 1, times = 5, source.times = "c", newobj = "short", datasources = conns)
ds.asList(x.name = "D$NUMERIC", newobj = "mylist", datasources = conns)
ds.dataFrame(c("D$NUMERIC", "D$INTEGER"), newobj = "numdf", datasources = conns)
ds.dataFrame(c("D$NUMERIC", "D$INTEGER"), newobj = "md1", datasources = conns[1])
ds.dataFrame(c("D$NUMERIC", "D$INTEGER", "D$LOGICAL"), newobj = "md1", datasources = conns[2:3])
ds.dataFrame(c("D$NUMERIC", "D$INTEGER"), newobj = "md2", datasources = conns[1])
ds.dataFrame(c("D$NUMERIC", "D$LOGICAL"), newobj = "md2", datasources = conns[2:3])


# ---- Shared helpers ----

# .checkArgsSet
ds.rBinom(samp.size = NULL, newobj = "x", datasources = conns)

# .resolveNewobjName / .checkNewobjName
ds.asNumeric(x.name = "D$NUMERIC", newobj = 1:2, datasources = conns)

# .seedAsText
ds.rNorm(seed.as.integer = "abc", newobj = "x", datasources = conns)

# .abortInvalidChoice
ds.rowColCalc("D", operation = "bogus", datasources = conns)
ds.rowColCalc("D", datasources = conns)
ds.getWGSR(sex = "D$LOGICAL", firstPart = "D$NUMERIC", secondPart = "D$NUMERIces = conns")
ds.Boole("D$NUMERIC", "D$NUMERIC", Boolean.operator = "~", datasources = conns)
ds.forestplot(ds.glmSLMA("D$NUMERIC~D$INTEGER", family = "gaussian", datasources = conns), variable = "zzz")

# .expandToStudies
ds.rNorm(mean = c(1, 2), newobj = "x", datasources = conns)

# .checkOffsetInFormula
ds.glm("D$NUMERIC~D$INTEGER+offset(D$POSITIVE_NUMERIC)", family = "gaussian", datasources = conns)

stency (via ds.length)
ds.length("mixednum", datasources = conns)

# .checkObjectClasses (via ds.rbind)
ds.rbind("mylist", DataSHIELD.checks = TRUE, datasources = conns)

# .checkMultiObjectConsistency (via ds.cbind)
ds.cbind(x = c("D", "D"), DataSHIELD.checks = TRUE, datasources = conns)
ds.cbind(x = c("D", "short"), DataSHIELD.checks = TRUE, datasources = conns)

# ---- glmChecks / isDefined ----
ds.glm("nope~D$INTEGER", family = "gaussian", checks = TRUE, datasources = conns[1])
ds.glm("D$NOPECOLUMN~D$INTEGER", family = "gaussian", checks = TRUE, datasources = conns[1])
ds.glm("D$NA_VALUES~D$INTEGER", family = "gaussian", checks = TRUE, datasources = conns[1])
ds.glm("D$NUMERIC~D$INTEGER", family = "gaussian", offset = "D$CHARACTER", checks = TRUE, datasources = conns[1])
ds.glm("D$NUMERIC~D$INTEGER", family = "gaussian", weights = "D$NEGATIVE_NUMERIC", checks = TRUE, datasources = conns[1])


# ---- Models ----

# ds.glm non-convergence warning
ds.glm("D$NUMERIC~D$INTEGER", family = "gaussian", maxit = 1, datasources = conns)
       
# ds.glmSummary: x.name not a string
ds.glmSummary(x.name = 1, datasources = conns)

# ds.lmerSLMA / ds.glmerSLMA: control_value missing
ds.lmerSLMA("D$NUMERIC~D$INTEGER+(1|D$FACTOR_INTEGER)", control_type = "xtol_rel", datasources = conns)
ds.glmerSLMA("D$LOGICAL~D$INTEGER+(1|D$FACTOR_INTEGER)", family = "binomial", control_type = "xtol_rel", datasources = conns)

# ds.gamlss: xvar missing with centiles
ds.gamlss(formula = "D$NUMERIC~D$INTEGER", centiles = TRUE, datasources = conns)


# ---- Random numbers and sampling ----

# "No seed set" 
ds.rNorm(newobj = "x", datasources = conns)

# setSeedDS progress message 
ds.sample(x = "D", size = 10, seed.as.integer = 27, notify.of.progress = TRUE, datasources = conns)

# Range checks
ds.rBinom(samp.size = 10, size = -1, prob = 0.5, newobj = "x", datasources = conns)
ds.rBinom(samp.size = 10, size = 1, prob = 2, newobj = "x", datasources = conns)
ds.rNorm(samp.size = 10, sd = -1, newobj = "x", datasources = conns)
ds.rNorm(samp.size = 10, force.output.to.k.decimal.places = 10, newobj = "x", datasources = conns)
ds.rPois(samp.size = 10, lambda = 0, newobj = "x", datasources = conns)
ds.rUnif(samp.size = 10, min = 2, max = 1, newobj = "x", datasources = conns)
ds.rUnif(samp.size = 10, force.output.to.k.decimal.places = 10, newobj = "x", datasources = conns)

# ds.sample: size missing
ds.sample(x = "D", datasources = conns)

# ---- Argument type checks ----
ds.getWGSR(sex = "D$LOGICAL", firstPart = "D$NUMERIC", secondPart = "D$NUMERIC", index = "bfa", datasources = conns)
ds.getWGSR(sex = "D$LOGICAL", firstPart = "D$NUMERIC", secondPart = "D$NUMERIC", index = "wfa", standing = 4, datasources = conns)

# ds.kurtosis / ds.skewness
ds.kurtosis("D$NUMERIC", method = 4, datasources = conns)
ds.skewness("D$NUMERIC", method = 4, datasources = conns)
ds.kurtosis("D$NA_VALUES", type = "combine", datasources = conns)
ds.skewness("D$NA_VALUES", type = "combine", datasources = conns)

# ds.tapply / ds.tapply.assign
ds.tapply(X.name = "D$NUMERIC", FUN.name = "mean", datasources = conns)
ds.tapply.assign(X.name = "D$NUMERIC", FUN.name = "mean", newobj = "t", datasources = conns)
ds.tapply("D$NUMERIC", INDEX.names = c("a", "b", "c"), FUN.name = "mean", datasources = conns)
ds.tapply.assign("D$NUMERIC", INDEX.names = c("a", "b", "c"), FUN.name = "mean", newobj = "t", datasources = conns)

# ds.dataFrameSubset
ds.dataFrameSubset(df.name = "D", V2.name = "D$INTEGER", Boolean.operator = "==", newobj = "s", datasources = conns)
ds.dataFrameSubset(df.name = "D", V1.name = "D$INTEGER", Boolean.operator = "==", newobj = "s", datasources = conns)
ds.dataFrameSubset(df.name = "D", V1.name = "D$INTEGER", V2.name = "D$INTEGER", newobj = "s", datasources = conns)

# ds.recodeValues
ds.recodeValues("D$INTEGER", values2replace.vector = c(1, 2), datasources = conns)
ds.recodeValues(var.name = "D$INTEGER", new.values.vector = 1, newobj = "r", datasources = conns)
ds.recodeValues("D$INTEGER", values2replace.vector = c(1, 2), new.values.vector = 1, datasources = conns)

ds.changeRefGroup("D$FACTOR_CHARACTER", ref = "divorced", reorderByRef = TRUE, datasources = conns)
ds.cor(x = "numdf", y = "D$NUMERIC", datasources = conns)
ds.cov(x = "numdf", y = "D$NUMERIC", datasources = conns)
ds.histogram("D$NUMERIC", type = "split", num.breaks = 20, datasources = conns)
ds.histogram("D$NUMERIC", num.breaks = 20, datasources = conns[1])

# ---- ds.mdPattern ----
ds.mdPattern("md1", type = "combine", datasources = conns)
ds.mdPattern("md2", type = "combine", datasources = conns)

# ---- ds.table ----
ds.table("D$FACTOR_INTEGER", cvar = 30, datasources = conns)
ds.table("D$FACTOR_INTEGER", stvar = 30, datasources = conns)
ds.table("D$FACTOR_INTEGER", force.nfilter = 5, datasources = conns)
ds.table("D$FACTOR_INTEGER", datasources = conns)
ds.table("D$NA_VALUES", datasources = conns)
ds.table("D$FACTOR_INTEGER", "D$NA_VALUES", datasources = conns)
ds.table("D$FACTOR_INTEGER", "D$LOGICAL", "D$NA_VALUES", datasources = conns)

github-actions · 2026-11-05T10:48:10Z

⏳ 4 of 5 checks reported so far

Check	Result
Devtools checks	✅ passed
Code quality	✅ 0 new findings (package total: 1891)
Armadillo unit tests	✅ `[ FAIL 0 \| WARN 0 \| SKIP 21 \| PASS 11611 ]`
Opal unit tests	✅ `[ FAIL 0 \| WARN 0 \| SKIP 21 \| PASS 11289 ]`
Test coverage	ℹ️ no coverable lines changed (project: 71.0%)

Tested against dsBase versions:
Armadillo: 7.0.0.9000
Opal: 7.0.0.9000

Logs: Devtools checks · Code quality · Armadillo unit tests · Opal unit tests · Codecov

github-actions · 2026-11-05T10:48:10Z

⏳ 0 of 5 checks reported so far

Check	Result
Devtools checks	⏳ pending
Code quality	⏳ pending
Armadillo unit tests	⏳ pending
Opal unit tests	⏳ pending
Test coverage	⏳ pending

Tested against dsBase versions:
Armadillo: pending
Opal: pending

Logs: pending · pending · pending · pending · pending

github-advanced-security

lintr found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

StuartWheater

It appears some tests are failing
- One possible reason appears to be test timeouts
Done - switch to '.resolveDatasources' to '.set_datasources'
Warnings about "::", like much of the code

codecov-commenter · 2026-11-13T11:05:41Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.05%. Comparing base (3bb4a0a) to head (d7250e7).

Additional details and impacted files

@@              Coverage Diff              @@
##           v7.0-dev     #731       +/-   ##
=============================================
+ Coverage     58.80%   71.05%   +12.25%     
=============================================
  Files           131      131               
  Lines          4685     4702       +17     
=============================================
+ Hits           2755     3341      +586     
+ Misses         1930     1361      -569

Flag	Coverage Δ
armadillo-arg	`32.32% <ø> (?)`
armadillo-misc	`19.94% <ø> (+0.37%)`	⬆️
armadillo-perf-1	`11.46% <ø> (+<0.01%)`	⬆️
armadillo-perf-2	`20.73% <ø> (?)`
armadillo-perf-3	`19.22% <ø> (-0.16%)`	⬇️
armadillo-smk-1	`43.78% <ø> (-0.01%)`	⬇️
armadillo-smk-2	`37.72% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Use cli for client messages

983677d

github-advanced-security Bot found potential problems Nov 5, 2026

View reviewed changes

Add context to cli messages

fedc6a7

github-advanced-security Bot found potential problems Nov 5, 2026

View reviewed changes

Comment thread R/utils.R Fixed

Thread call context through checkClass, isDefined and glmChecks

dbb0b34

github-advanced-security Bot found potential problems Nov 6, 2026

View reviewed changes

Comment thread R/glmChecks.R Fixed

Comment thread R/glmChecks.R Fixed

Comment thread R/glmChecks.R Fixed

Comment thread R/glmChecks.R Fixed

Comment thread R/glmChecks.R Fixed

Comment thread R/glmChecks.R Fixed

Show supplied values in cli messages

e452d3e

github-advanced-security Bot found potential problems Nov 9, 2026

View reviewed changes

timcadman added 3 commits November 9, 2026 14:49

Tighten error message assertions

0ef8077

Fix lint findings

f0fb5a6

Drop testthat:: prefix in tests

15a77f0

github-advanced-security Bot found potential problems Nov 9, 2026

View reviewed changes

Comment thread tests/testthat/test-arg-error-messages.R Fixed

Comment thread tests/testthat/test-arg-error-messages.R Fixed

Comment thread tests/testthat/test-arg-per-study-arguments.R Fixed

timcadman added 3 commits November 10, 2026 09:44

Restore testthat:: prefix on local_mocked_bindings

0ec9d5b

Suppress lintr false positive on local_mocked_bindings

1d140e0

Regenerate docs for call parameter

9c04a75

StuartWheater approved these changes Nov 11, 2026

View reviewed changes

timcadman marked this pull request as ready for review November 13, 2026 10:12

timcadman added 2 commits November 13, 2026 12:38

Merge branch 'v7.0-dev' into refactor/cli-messages

a270c92

disable arrow threads in Armadillo test data upload

b2325ad

github-advanced-security Bot found potential problems Nov 13, 2026

View reviewed changes

Comment thread tests/testthat/data_files/molgenis_armadillo-upload_testing_datasets.R Fixed

exclude arrow options line from undesirable_function_linter

d7250e7

Conversation

timcadman commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backgound

What's changed

How to test

Uh oh!

github-actions Bot commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Nov 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-advanced-security AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

StuartWheater left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Nov 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

timcadman commented Nov 5, 2026 •

edited

Loading

github-actions Bot commented Nov 5, 2026 •

edited

Loading

codecov-commenter commented Nov 13, 2026 •

edited

Loading