Skip to content

Refactor/perf batch 6 #696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
timcadman wants to merge 12 commits into
base: v7.0-dev
Choose a base branch
from
Draft

Refactor/perf batch 6 #696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
981ac60
chore: bootstrap batch-6 from batch-5
timcadman Apr 15, 2026
501abec
refactor: batch-6 factor and recoding client functions
timcadman Apr 15, 2026
12f85f7
chore: update dsBase tar with batch-6
timcadman Apr 15, 2026
fda3ead
test: add performance tests for batch 6 functions
timcadman Apr 16, 2026
a904692
fix: remove third-party copyright from new tests
timcadman Apr 16, 2026
41189a0
fix: escaped dollar signs in perf tests
timcadman Apr 16, 2026
37ba40c
fix: correct perf test function calls for batch 6
timcadman Apr 16, 2026
f4cf555
docs: sync REFACTOR_GUIDE.md from batch-9
timcadman Apr 16, 2026
937d3e1
docs: sync REFACTOR_GUIDE.md from batch-10
timcadman Apr 16, 2026
fdbb0ad
fix: align asFactor and changeRefGroup perf tests with smoke tests
timcadman Apr 19, 2026
4ac9eb9
fix: align perf test datasets with smoke tests
timcadman Apr 19, 2026
30528a1
docs: sync REFACTOR_GUIDE.md
timcadman Apr 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .Rbuildignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@
^\.circleci$
^\.circleci/config\.yml$
^\.github$
^REFACTOR_GUIDE\.md$
2 changes: 2 additions & 0 deletions NAMESPACE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ export(ds.var)
export(ds.vectorCalc)
import(DSI)
import(data.table)
importFrom(DSI,datashield.connections_find)
importFrom(cli,cli_abort)
importFrom(stats,as.formula)
importFrom(stats,na.omit)
importFrom(stats,ts)
Expand Down
70 changes: 5 additions & 65 deletions R/ds.asFactor.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,8 @@
#' @param datasources a list of \code{\link[DSI]{DSConnection-class}} objects obtained after login.
#' If the \code{datasources} argument is not specified
#' the default set of connections will be used: see \code{\link[DSI]{datashield.connections_default}}.
#' @return \code{ds.asFactor} returns the unique levels of the converted
#' variable in ascending order and a validity
#' message with the name of the created object on the client-side and
#' the output matrix or vector in the server-side.
#' @return \code{ds.asFactor} returns the unique levels of the converted
#' variable in ascending order. The output matrix or vector is written to the server-side.
#'
#' @examples
#' \dontrun{
Expand Down Expand Up @@ -185,19 +183,12 @@
#' datashield.logout(connections)
#' }
#' @author DataSHIELD Development Team
#' @author Tim Cadman, Genomics Coordination Centre, UMCG, Netherlands
#' @export
ds.asFactor <- function(input.var.name=NULL, newobj.name=NULL, forced.factor.levels=NULL, fixed.dummy.vars=FALSE,
baseline.level=1, datasources=NULL){

# look for DS connections
if(is.null(datasources)){
datasources <- datashield.connections_find()
}

# ensure datasources is a list of DSConnection-class
if(!(is.list(datasources) && all(unlist(lapply(datasources, function(d) {methods::is(d,"DSConnection")}))))){
stop("The 'datasources' were expected to be a list of DSConnection-class objects", call.=FALSE)
}
datasources <- .set_datasources(datasources)

# check if user has provided the name of the column that holds the input variable
if(is.null(input.var.name)){
Expand Down Expand Up @@ -248,58 +239,7 @@ ds.asFactor <- function(input.var.name=NULL, newobj.name=NULL, forced.factor.lev
calltext2 <- call("asFactorDS2", input.var.name, all.unique.levels.transmit, fixed.dummy.vars, baseline.level)
DSI::datashield.assign(datasources, newobj.name, calltext2)

##########################################################################################################
#MODULE 5: CHECK KEY DATA OBJECTS SUCCESSFULLY CREATED #
#
#SET APPROPRIATE PARAMETERS FOR THIS PARTICULAR FUNCTION #
test.obj.name<-newobj.name #
#
# CALL SEVERSIDE FUNCTION #
calltext <- call("testObjExistsDS", test.obj.name) #
object.info<-DSI::datashield.aggregate(datasources, calltext) #
#
# CHECK IN EACH SOURCE WHETHER OBJECT NAME EXISTS #
# AND WHETHER OBJECT PHYSICALLY EXISTS WITH A NON-NULL CLASS #
num.datasources<-length(object.info) #
#
#
obj.name.exists.in.all.sources<-TRUE #
obj.non.null.in.all.sources<-TRUE #
#
for(j in 1:num.datasources){ #
if(!object.info[[j]]$test.obj.exists){ #
obj.name.exists.in.all.sources<-FALSE #
} #
if(is.null(object.info[[j]]$test.obj.class) || ("ABSENT" %in% object.info[[j]]$test.obj.class)){ #
obj.non.null.in.all.sources<-FALSE #
} #
} #
#
if(obj.name.exists.in.all.sources && obj.non.null.in.all.sources){ #
#
return.message<- #
paste0("Data object <", test.obj.name, "> correctly created in all specified data sources") #
#
return(list(all.unique.levels=all.unique.levels,return.message=return.message)) #
#
}else{ #
#
return.message.1<- #
paste0("Error: A valid data object <", test.obj.name, "> does NOT exist in ALL specified data sources")#
#
return.message.2<- #
paste0("It is either ABSENT and/or has no valid content/class,see return.info above") #
#
return.message<-list(return.message.1,return.message.2) #
#
return.info<-object.info #
#
return(list(all.unique.levels=all.unique.levels,return.info=return.info,return.message=return.message)) #
#
} #
#END OF MODULE 5 #
##########################################################################################################

return(list(all.unique.levels=all.unique.levels))

}
#ds.asFactor
68 changes: 3 additions & 65 deletions R/ds.asFactorSimple.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,14 @@
#' @param datasources a list of \code{\link[DSI]{DSConnection-class}} objects obtained after login.
#' If the \code{datasources} argument is not specified
#' the default set of connections will be used: see \code{\link[DSI]{datashield.connections_default}}.
#' @return an output vector of class factor to the serverside. In addition, returns a validity
#' message with the name of the created object on the client-side and if creation fails an
#' error message which can be viewed using datashield.errors().
#' @return an output vector of class factor written to the serverside.
#' @author DataSHIELD Development Team
#' @author Tim Cadman, Genomics Coordination Centre, UMCG, Netherlands
#' @export
#'
ds.asFactorSimple <- function(input.var.name=NULL, newobj.name=NULL, datasources=NULL){

# look for DS connections
if(is.null(datasources)){
datasources <- datashield.connections_find()
}

# ensure datasources is a list of DSConnection-class
if(!(is.list(datasources) && all(unlist(lapply(datasources, function(d) {methods::is(d,"DSConnection")}))))){
stop("The 'datasources' were expected to be a list of DSConnection-class objects", call.=FALSE)
}
datasources <- .set_datasources(datasources)

# check if user has provided the name of the column that holds the input variable
if(is.null(input.var.name)){
Expand All @@ -55,58 +46,5 @@ ds.asFactorSimple <- function(input.var.name=NULL, newobj.name=NULL, datasources
calltext0 <- call("asFactorSimpleDS", input.var.name)
DSI::datashield.assign(datasources, newobj.name, calltext0)

##########################################################################################################
#MODULE 5: CHECK KEY DATA OBJECTS SUCCESSFULLY CREATED #
#
#SET APPROPRIATE PARAMETERS FOR THIS PARTICULAR FUNCTION #
test.obj.name<-newobj.name #
#
# CALL SEVERSIDE FUNCTION #
calltext <- call("testObjExistsDS", test.obj.name) #
object.info<-DSI::datashield.aggregate(datasources, calltext) #
#
# CHECK IN EACH SOURCE WHETHER OBJECT NAME EXISTS #
# AND WHETHER OBJECT PHYSICALLY EXISTS WITH A NON-NULL CLASS #
num.datasources<-length(object.info) #
#
#
obj.name.exists.in.all.sources<-TRUE #
obj.non.null.in.all.sources<-TRUE #
#
for(j in 1:num.datasources){ #
if(!object.info[[j]]$test.obj.exists){ #
obj.name.exists.in.all.sources<-FALSE #
} #
if(is.null(object.info[[j]]$test.obj.class) || ("ABSENT" %in% object.info[[j]]$test.obj.class)){ #
obj.non.null.in.all.sources<-FALSE #
} #
} #
#
if(obj.name.exists.in.all.sources && obj.non.null.in.all.sources){ #
#
return.message<- #
paste0("Data object <", test.obj.name, "> correctly created in all specified data sources") #
#
return(list(return.message=return.message)) #
#
}else{ #
#
return.message.1<- #
paste0("Error: A valid data object <", test.obj.name, "> does NOT exist in ALL specified data sources")#
#
return.message.2<- #
paste0("It is either ABSENT and/or has no valid content/class,see return.info above") #
#
return.message<-list(return.message.1,return.message.2) #
#
return.info<-object.info #
#
return(list(return.info=return.info,return.message=return.message)) #
#
} #
#END OF MODULE 5 #
##########################################################################################################


}
#ds.asFactorSimple
21 changes: 4 additions & 17 deletions R/ds.changeRefGroup.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#' @return \code{ds.changeRefGroup} returns a new vector with the specified level as a reference
#' which is written to the server-side.
#' @author DataSHIELD Development Team
#' @author Tim Cadman, Genomics Coordination Centre, UMCG, Netherlands
#' @seealso \code{\link{ds.cbind}} Combines objects column-wise.
#' @seealso \code{\link{ds.levels}} to obtain the levels (categories) of a vector of type factor.
#' @seealso \code{\link{ds.colnames}} to obtain the column names of a matrix or a data frame
Expand Down Expand Up @@ -109,15 +110,7 @@
#' @export
ds.changeRefGroup <- function(x=NULL, ref=NULL, newobj=NULL, reorderByRef=FALSE, datasources=NULL){

# look for DS connections
if(is.null(datasources)){
datasources <- datashield.connections_find()
}

# ensure datasources is a list of DSConnection-class
if(!(is.list(datasources) && all(unlist(lapply(datasources, function(d) {methods::is(d,"DSConnection")}))))){
stop("The 'datasources' were expected to be a list of DSConnection-class objects", call.=FALSE)
}
datasources <- .set_datasources(datasources)

if(is.null(x)){
stop("Please provide the name of a vector of type factor!", call.=FALSE)
Expand All @@ -132,9 +125,6 @@ ds.changeRefGroup <- function(x=NULL, ref=NULL, newobj=NULL, reorderByRef=FALSE,
newobj <- "changerefgroup.newobj"
}

# check if the input object is defined in all the studies
isDefined(datasources, x)

# call the internal function that checks the input object is of the same class in all studies.
typ <- checkClass(datasources, x)

Expand All @@ -148,10 +138,7 @@ ds.changeRefGroup <- function(x=NULL, ref=NULL, newobj=NULL, reorderByRef=FALSE,
}

# call the server side function that will recode the levels
cally <- paste0('changeRefGroupDS(', x, ",'", ref, "',", reorderByRef,")")
DSI::datashield.assign(datasources, newobj, as.symbol(cally))

# check that the new object has been created and display a message accordingly
finalcheck <- isAssigned(datasources, newobj)
calltext <- call("changeRefGroupDS", x, ref, reorderByRef)
DSI::datashield.assign(datasources, newobj, calltext)

}
13 changes: 3 additions & 10 deletions R/ds.dmtC2S.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,13 @@
#' @return the object specified by the <newobj> argument (or default name "dmt.copied.C2S")
#' which is written as a data.frame/matrix/tibble to the serverside.
#' @author Paul Burton for DataSHIELD Development Team - 3rd June, 2021
#' @author Tim Cadman, Genomics Coordination Centre, UMCG, Netherlands
#' @export
#'
ds.dmtC2S <- function(dfdata=NA, newobj=NULL, datasources=NULL){

# if no opal login details are provided look for 'opal' objects in the environment
if(is.null(datasources)){
datasources <- datashield.connections_find()
}

# ensure datasources is a list of DSConnection-class
if(!(is.list(datasources) && all(unlist(lapply(datasources, function(d) {methods::is(d,"DSConnection")}))))){
stop("The 'datasources' were expected to be a list of DSConnection-class objects", call.=FALSE)
}

datasources <- .set_datasources(datasources)

# check if a value has been provided for dfdata
if(is.null(dfdata)){
return("Error: dfdata must be a character string, a numeric vector or a scalar")
Expand Down
94 changes: 4 additions & 90 deletions R/ds.reShape.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,10 @@
#' @param datasources a list of \code{\link[DSI]{DSConnection-class}}
#' objects obtained after login. If the \code{datasources} argument is not specified
#' the default set of connections will be used: see \code{\link[DSI]{datashield.connections_default}}.
#' @return \code{ds.reShape} returns to the server-side a reshaped data frame
#' converted from 'long' to 'wide' format or from 'wide' to long' format.
#' Also, two validity messages are returned to the client-side
#' indicating whether the new object has been created in each data source and if so whether
#' it is in a valid form.
#' @return \code{ds.reShape} returns to the server-side a reshaped data frame
#' converted from 'long' to 'wide' format or from 'wide' to long' format.
#' @author DataSHIELD Development Team
#' @author Tim Cadman, Genomics Coordination Centre, UMCG, Netherlands
#' @examples
#' \dontrun{
#'
Expand Down Expand Up @@ -84,15 +82,7 @@
ds.reShape <- function(data.name=NULL, varying=NULL, v.names=NULL, timevar.name="time", idvar.name="id",
drop=NULL, direction=NULL, sep=".", newobj="newObject", datasources=NULL){

# look for DS connections
if(is.null(datasources)){
datasources <- datashield.connections_find()
}

# ensure datasources is a list of DSConnection-class
if(!(is.list(datasources) && all(unlist(lapply(datasources, function(d) {methods::is(d,"DSConnection")}))))){
stop("The 'datasources' were expected to be a list of DSConnection-class objects", call.=FALSE)
}
datasources <- .set_datasources(datasources)

if(is.null(data.name)){
stop("Please provide the name of the list that holds the input vectors!", call.=FALSE)
Expand Down Expand Up @@ -125,81 +115,5 @@ ds.reShape <- function(data.name=NULL, varying=NULL, v.names=NULL, timevar.name=
calltext <- call("reShapeDS", data.name, varying.transmit, v.names.transmit, timevar.name, idvar.name, drop.transmit, direction, sep)
DSI::datashield.assign(datasources, newobj, calltext)

#############################################################################################################
#DataSHIELD CLIENTSIDE MODULE: CHECK KEY DATA OBJECTS SUCCESSFULLY CREATED #
#
#SET APPROPRIATE PARAMETERS FOR THIS PARTICULAR FUNCTION #
test.obj.name<-newobj #
# #
#
# CALL SEVERSIDE FUNCTION #
calltext <- call("testObjExistsDS", test.obj.name) #
#
object.info<-DSI::datashield.aggregate(datasources, calltext) #
#
# CHECK IN EACH SOURCE WHETHER OBJECT NAME EXISTS #
# AND WHETHER OBJECT PHYSICALLY EXISTS WITH A NON-NULL CLASS #
num.datasources<-length(object.info) #
#
#
obj.name.exists.in.all.sources<-TRUE #
obj.non.null.in.all.sources<-TRUE #
#
for(j in 1:num.datasources){ #
if(!object.info[[j]]$test.obj.exists){ #
obj.name.exists.in.all.sources<-FALSE #
} #
if(is.null(object.info[[j]]$test.obj.class) || ("ABSENT" %in% object.info[[j]]$test.obj.class)){ #
obj.non.null.in.all.sources<-FALSE #
} #
} #
#
if(obj.name.exists.in.all.sources && obj.non.null.in.all.sources){ #
#
return.message<- #
paste0("A data object <", test.obj.name, "> has been created in all specified data sources") #
#
#
}else{ #
#
return.message.1<- #
paste0("Error: A valid data object <", test.obj.name, "> does NOT exist in ALL specified data sources") #
#
return.message.2<- #
paste0("It is either ABSENT and/or has no valid content/class,see return.info above") #
#
return.message.3<- #
paste0("Please use ds.ls() to identify where missing") #
#
#
return.message<-list(return.message.1,return.message.2,return.message.3) #
#
} #
#
calltext <- call("messageDS", test.obj.name) #
studyside.message<-DSI::datashield.aggregate(datasources, calltext) #
#
no.errors<-TRUE #
for(nd in 1:num.datasources){ #
if(studyside.message[[nd]]!="ALL OK: there are no studysideMessage(s) on this datasource"){ #
no.errors<-FALSE #
} #
} #
#
#
if(no.errors){ #
validity.check<-paste0("<",test.obj.name, "> appears valid in all sources") #
return(list(is.object.created=return.message,validity.check=validity.check)) #
} #
#
if(!no.errors){ #
validity.check<-paste0("<",test.obj.name,"> invalid in at least one source. See studyside.messages:") #
return(list(is.object.created=return.message,validity.check=validity.check, #
studyside.messages=studyside.message)) #
} #
#
#END OF CHECK OBJECT CREATED CORECTLY MODULE #
#############################################################################################################

}
#ds.reShape
Loading