-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
GH-15894: Make sure the functions that are supposed to be exported are exported in the R package #15899
GH-15894: Make sure the functions that are supposed to be exported are exported in the R package #15899
Changes from all commits
39560c4
8f3fb41
0d2c340
78beec4
6cfe711
215ce15
5fbc420
61bd02a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,13 @@ | |
registerS3method("print", "H2OExplanation", "print.H2OExplanation") | ||
.s3_register("repr", "repr_text", "H2OExplanation") | ||
.s3_register("repr", "repr_html", "H2OExplanation") | ||
|
||
# CoxPH | ||
.s3_register("survival", "survfit", "H2OCoxPHModel") | ||
|
||
# H2OFrame | ||
.s3_register("stats", "median", "H2OFrame") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Confusingly, |
||
|
||
invisible() | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -154,8 +154,8 @@ reference: | |
- h2o.get_best_model_predictors | ||
- h2o.get_knot_locations | ||
- h2o.get_gam_knot_column_names | ||
- h2o.get_predictor_added_per_step | ||
- h2o.get_predictor_removed_per_step | ||
- h2o.get_predictors_added_per_step | ||
- h2o.get_predictors_removed_per_step | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (missing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! |
||
- h2o.get_regression_influence_diagnostics | ||
- h2o.get_variable_inflation_factors | ||
- h2o.glm | ||
|
@@ -333,8 +333,6 @@ reference: | |
- h2o.tanh | ||
- h2o.target_encode_apply | ||
- h2o.target_encode_create | ||
- h2o.target_encode_fit | ||
- h2o.target_encode_transform | ||
- h2o.targetencoder | ||
- h2o.tf_idf | ||
- h2o.toFrame | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -182,16 +182,28 @@ function() { | |
strict_version_check <- TRUE | ||
} else if (IS.RUNIT) { | ||
# source h2o-r/h2o-package/R. overrides h2o package load | ||
to_src <- c("aggregator.R", "classes.R", "connection.R","config.R", "constants.R", "logging.R", "communication.R", | ||
"kvstore.R", "frame.R", "targetencoder.R", "astfun.R","automl.R", "import.R", "parse.R", "export.R", "models.R", "edicts.R", | ||
"coxph.R", "coxphutils.R", "gbm.R", "glm.R", "gam.R", "anovaglm.R", "glrm.R", "kmeans.R", "deeplearning.R", "randomforest.R", "generic.R", | ||
"naivebayes.R", "pca.R", "svd.R", "locate.R", "grid.R", "word2vec.R", "w2vutils.R", "stackedensemble.R", "rulefit.R", "modelselection.R", | ||
"predict.R", "xgboost.R", "isolationforest.R", "psvm.R", "segment.R", "tf-idf.R", "explain.R", "permutation_varimp.R", "extendedisolationforest.R", | ||
"upliftrandomforest.R", "infogram.R", "isotonicregression.R", "admissibleml.R", "decisiontree.R", "adaboost.R") | ||
|
||
src_path <- paste(h2oRDir,"h2o-package","R",sep=.Platform$file.sep) | ||
invisible(lapply(to_src,function(x){source(paste(src_path, x, sep = .Platform$file.sep))})) | ||
# to_src <- c("aggregator.R", "classes.R", "connection.R","config.R", "constants.R", "logging.R", "communication.R", | ||
# "kvstore.R", "frame.R", "targetencoder.R", "astfun.R","automl.R", "import.R", "parse.R", "export.R", "models.R", "edicts.R", | ||
# "coxph.R", "coxphutils.R", "gbm.R", "glm.R", "gam.R", "anovaglm.R", "glrm.R", "kmeans.R", "deeplearning.R", "randomforest.R", "generic.R", | ||
# "naivebayes.R", "pca.R", "svd.R", "locate.R", "grid.R", "word2vec.R", "w2vutils.R", "stackedensemble.R", "rulefit.R", "modelselection.R", | ||
# "predict.R", "xgboost.R", "isolationforest.R", "psvm.R", "segment.R", "tf-idf.R", "explain.R", "permutation_varimp.R", "extendedisolationforest.R", | ||
# "upliftrandomforest.R", "infogram.R", "isotonicregression.R", "admissibleml.R", "decisiontree.R", "adaboost.R") | ||
|
||
# src_path <- paste(h2oRDir,"h2o-package","R",sep=.Platform$file.sep) | ||
# invisible(lapply(to_src,function(x){source(paste(src_path, x, sep = .Platform$file.sep))})) | ||
devtools::load_all(paste(h2oRDir,"h2o-package",sep=.Platform$file.sep), export_all=FALSE) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
additional_imports <- c( | ||
maurever marked this conversation as resolved.
Show resolved
Hide resolved
|
||
".h2o.__remoteSend", ".as.survival.coxph.model", ".as.survival.coxph.summary", | ||
".getExpanded", ".str.list", "is.H2OFrame", ".get.session.property", ".set.session.property", | ||
".h2o.maximizing_metrics", ".h2o.doSafeGET", ".parse.h2oconfig", ".h2o.check_java_version", | ||
"cut.H2OFrame", "as.data.frame.H2OFrame", ".h2o.perfect_auc", ".newExpr", ".h2o.doSafeREST", | ||
".h2o.fromJSON" | ||
) | ||
|
||
for (fn in additional_imports) | ||
assign(fn, do.call(`:::`, list(quote(h2o), fn)), envir = .GlobalEnv) | ||
|
||
# source h2o-r/tests/runitUtils | ||
to_src <- c("utilsR.R", "pcaR.R", "deeplearningR.R", "glmR.R", "glrmR.R", | ||
"gbmR.R", "kmeansR.R", "naivebayesR.R", "gridR.R", "shared_javapredict.R") | ||
|
@@ -209,7 +221,7 @@ function() { | |
masterSeedFile <- paste(getwd(), "/master_seed", sep = "") | ||
if (file.exists(masterSeedFile)) seed <- read.table(masterSeedFile)[[1]] | ||
setupSeed(seed) | ||
h2o.logIt("[SEED] :", SEED) | ||
cat("[SEED] :", SEED, "\n") | ||
|
||
if (!is.na(USERNAME)) { | ||
Log.info(paste0("Authenticating with http-basic, username: ", USERNAME, ".")) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,10 +113,13 @@ function(cur.dir, root, root.parent = NULL) { | |
default.packages <- | ||
function() { | ||
to_require <- c("jsonlite", "RCurl", "RUnit", "R.utils", "testthat", "ade4", "glmnet", "gbm", "ROCR", "e1071", | ||
"tools", "statmod", "fpc", "cluster") | ||
"tools", "statmod", "fpc", "cluster", "survival") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for CoxPH tests |
||
if (Sys.info()['sysname'] == "Windows") { | ||
options(RCurlOptions = list(cainfo = system.file("CurlSSL", "cacert.pem", package = "RCurl"))) } | ||
invisible(lapply(to_require,function(x){require(x,character.only=TRUE,quietly=TRUE,warn.conflicts=FALSE)})) | ||
invisible(lapply(to_require,function(x) { | ||
if (!require(x,character.only=TRUE,quietly=TRUE,warn.conflicts=FALSE)) | ||
warning(paste("Package", x, "is not available."), call. = FALSE) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mainly to make it easier for us to find out why some test might not work. We import packages and if unsuccessful we used to silently fail, after this change we should see that some package is not available (and R names are sometimes quite obscure so it's hard to guess if we are missing a package or badly exporting something or both ( |
||
})) | ||
} | ||
|
||
read.zip<- | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Registering a class with S3 method in one package to a generic from another package is hard in R (unless it's some of the packages that are loaded by default in the package environment (
base
) - for those we can use theregisterS3Method
).