Skip to content

[SPARK-16144][SPARKR] update R API doc for mllib #13993

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

Closed
felixcheung wants to merge 2 commits into from
Closed

[SPARK-16144][SPARKR] update R API doc for mllib #13993

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
afc8ebe
update doc for mlliv
felixcheung Jun 30, 2016
1f9552e
update
felixcheung Jun 30, 2016
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
Prev Previous commit
update
  • Loading branch information
@felixcheung
felixcheung committed Jun 30, 2016
commit 1f9552ebd941d43da792c1fc6fdc461904a988fd
20 changes: 11 additions & 9 deletions R/pkg/R/mllib.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,26 +53,27 @@ setClass("AFTSurvivalRegressionModel", representation(jobj = "jobj"))
#' @note KMeansModel since 2.0.0
setClass("KMeansModel", representation(jobj = "jobj"))

#' Saves the machine learning model to the input path
#' Saves the MLlib model to the input path
Copy link
Contributor

@junyangq junyangq Jul 6, 2016

Choose a reason for hiding this comment

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

I would suggest shorter title like "Save Machine Learning Model"

Copy link
Member Author

@felixcheung felixcheung Jul 7, 2016

Choose a reason for hiding this comment

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

I think the convention that has been suggested is that we have the page title being the same first sentence of the description?

Copy link
Contributor

@junyangq junyangq Jul 9, 2016

Choose a reason for hiding this comment

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

I sort of have some impression, but would this be too restrictive? @shivaram

Copy link
Contributor

@yanboliang yanboliang Jul 10, 2016

Choose a reason for hiding this comment

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

The write.ml can only be used for saving MLlib models, it can not save other machine learning model produced by native R functions. So I think the current description is accurate enough.

#'
#' Saves the machine learning model to the input path. For more information, see the specific
#' machine learning model below.
#' Saves the MLlib model to the input path. For more information, see the specific
#' MLlib model below.
#' @rdname write.ml
#' @name write.ml
#' @export
#' @seealso \link{spark.glm}, \link{spark.kmeans}, \link{spark.naiveBayes}, \link{spark.survreg}
#' @seealso \link{spark.glm}, \link{glm}
#' @seealso \link{spark.kmeans}, \link{spark.naiveBayes}, \link{spark.survreg}
#' @seealso \link{read.ml}
Copy link
Contributor

@yanboliang yanboliang Jun 30, 2016

Choose a reason for hiding this comment

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

It's better to add @seealso \link{write.ml} in the docs of read.ml.

NULL

#' Predicted values based on a machine learning model
#' Makes predictions from a MLlib model
#'
#' Predicted values based on a machine learning model. For more information, see the specific
#' machine learning model below.
#' Makes predictions from a MLlib model. For more information, see the specific
Copy link
Contributor

@junyangq junyangq Jul 6, 2016

Choose a reason for hiding this comment

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

Makes -> Make?

Copy link
Member Author

@felixcheung felixcheung Jul 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Similarly, here, the "singular verb" with "s" is the convention. Please see eg. https://github.com/apache/spark/pull/13993/files#diff-7ede1519b4a56647801b51af33c2dd18R81

Copy link
Contributor

@junyangq junyangq Jul 9, 2016

Choose a reason for hiding this comment

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

Good catch - agreed.

#' MLlib model below.
#' @rdname predict
#' @name predict
#' @export
#' @seealso \link{spark.glm}, \link{spark.kmeans}, \link{spark.naiveBayes}, \link{spark.survreg}
#' @seealso \link{read.ml}
#' @seealso \link{spark.glm}, \link{glm}
#' @seealso \link{spark.kmeans}, \link{spark.naiveBayes}, \link{spark.survreg}
NULL

#' Generalized Linear Models
Expand Down Expand Up @@ -528,6 +529,7 @@ setMethod("write.ml", signature(object = "KMeansModel", path = "character"),
#' @rdname read.ml
#' @name read.ml
#' @export
#' @seealso \link{write.ml}
#' @examples
#' \dontrun{
#' path <- "path/to/model"
Expand Down