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

❌ 1 of 5 checks failed

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

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

timcadman added 3 commits October 30, 2026 14:16

Rename client helpers to camelCase and use newobj helper

cdc1cd6

Share argument checks in helpers and raise returned errors

b3ec216

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

Navigation Menu

Repository navigation

Refactor/cli messages - #731

Refactor/cli messages#731
timcadman wants to merge 12 commits into
v7.0-devfrom
refactor/cli-messages

timcadman commented Nov 5, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Nov 5, 2026 •

edited

Loading

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

Uh oh!

Uh oh!

Uh oh!

Uh oh!

StuartWheater left a comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

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!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

timcadman commented Nov 5, 2026 •

edited

Loading

github-actions Bot commented Nov 5, 2026 •

edited

Loading