Skip to content
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

GH-15894: Make sure the functions that are supposed to be exported are exported in the R package #15899

Merged
merged 8 commits into from
Nov 10, 2023
Merged

GH-15894: Make sure the functions that are supposed to be exported are exported in the R package #15899

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
39560c4
use devtools import into tests only the exported functions/methods
tomasfryda Nov 2, 2023
8f3fb41
Import more internal functions for tests
tomasfryda Nov 3, 2023
0d2c340
Remove non-existent target_encode functions from the _pkgdown.yml
tomasfryda Nov 3, 2023
78beec4
Export extractAIC for CoxPH and add more internal functions
tomasfryda Nov 6, 2023
6cfe711
Export median.H2OFrame
tomasfryda Nov 6, 2023
215ce15
Fix stats::median for H2OFrame
tomasfryda Nov 6, 2023
5fbc420
Improve docs and docstrings
tomasfryda Nov 7, 2023
61bd02a
Remove h2o.logIt
tomasfryda Nov 8, 2023
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 h2o-r/h2o-package/R/classes.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ coef.H2OCoxPHModelSummary <- function(object, ...) object@summary$coefficients
#' @param fit an \code{H2OCoxPHModel} object.
#' @param scale optional numeric specifying the scale parameter of the model.
#' @param k numeric specifying the weight of the equivalent degrees of freedom.
#' @export
extractAIC.H2OCoxPHModel <- function(fit, scale, k = 2, ...) {
fun <- get("extractAIC.coxph", getNamespace("stats"))
if (missing(scale))
Expand Down
10 changes: 10 additions & 0 deletions h2o-r/h2o-package/R/models.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,11 +755,21 @@ predict_leaf_node_assignment.H2OModel <- function(object, newdata, type = c("Pat
#' @export
h2o.predict_leaf_node_assignment <- predict_leaf_node_assignment.H2OModel

#' Use GRLM to transform a frame.
#'
#' @param model H2O GRLM model
#' @param fr H2OFrame
#' @return Retuns a transformed frame
#' @export
h2o.transform_frame <- function(model, fr) {
if (!is(model, "H2OModel") || (is(model, "H2OModel") && model@algorithm != "glrm")) stop("h2o.transform_frame can only be applied to GLRM H2OModel instance.")
return(.newExpr("transform", model@model_id, h2o.getId(fr)))
}

#' Retrieve the results to view the best predictor subsets
#' @param model modelselection object
#' @return Returns an H2OFrame object
#' @export
h2o.result <- function(model) {
if (!is(model, "H2OModel")) stop("h2o.result can only be applied to H2OModel instances with constant results")
return(.newExpr("result", model@model_id))
Expand Down
1 change: 1 addition & 0 deletions h2o-r/h2o-package/R/tf-idf.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#'
#' @return resulting frame with TF-IDF values.
#' Row format: documentID, word, TF, IDF, TF-IDF
#' @export
h2o.tf_idf <- function(frame, document_id_col, text_col, preprocess=TRUE, case_sensitive=TRUE) {
col_indices <- c()
for (col in c(document_id_col, text_col))
Expand Down
7 changes: 7 additions & 0 deletions h2o-r/h2o-package/R/zzz.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Contributor Author

@tomasfryda tomasfryda Nov 3, 2023 •
edited
Loading

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 the registerS3Method).


# H2OFrame
.s3_register("stats", "median", "H2OFrame")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stats package is not imported in the package namespace so it has to be specified here.

Confusingly, min, mean, max are defined in base package that is imported so they don't have to be listed here.


invisible()
}

Expand Down
6 changes: 2 additions & 4 deletions h2o-r/h2o-package/pkgdown/_pkgdown.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(missing s in predictorS )

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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
Expand Down
30 changes: 21 additions & 9 deletions h2o-r/scripts/h2o-r-test-setup.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export_all = FALSE tells devtools to just export those methods that are supposed to be exported so this helps to find out which methods are not exported but used in the tests.


additional_imports <- c(
maurever marked this conversation as resolved.
Show resolved Hide resolved
"h2o.logIt", ".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")
Expand Down
7 changes: 5 additions & 2 deletions h2o-r/tests/runitUtils/utilsR.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 (survfit.H2OCoxPHModel)).

}))
}

read.zip<-
Expand Down
2 changes: 1 addition & 1 deletion h2o-r/tests/testdir_algos/coxph/runit_coxph_cancer.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ test.CoxPH.cancer <- function() {
expect_equal(fit$var, .as.survival.coxph.model(hex.fit@model)$var, tolerance = 1e-8)
}

doTest("CoxPH: Cancer Test", test.CoxPH.cancer)
doTest("CoxPH: Cancer Test", test.CoxPH.cancer)