[SPARK-16140][MLlib][SparkR][Docs] Group k-means method in generated R doc #13921

keypointt · 2016-06-27T07:35:01Z

https://issues.apache.org/jira/browse/SPARK-16140

What changes were proposed in this pull request?

Group the R doc of spark.kmeans, predict(KM), summary(KM), read/write.ml(KM) under Rd spark.kmeans. The example code was updated.

How was this patch tested?

Tested on my local machine

And on my laptop jekyll build is failing to build API docs, so here I can only show you the html I manually generated from Rd files, with no CSS applied, but the doc content should be there.

SparkQA · 2016-06-27T08:12:43Z

Test build #61294 has finished for PR 13921 at commit 1228483.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

mengxr · 2016-06-27T17:19:04Z

@junyangq Could you help review this PR? Thanks!

junyangq · 2016-06-27T19:21:59Z

It would be better to group write.ml (KM) into the doc as well. Also please include a full screenshot of the doc page.

keypointt · 2016-06-27T19:42:58Z

R/pkg/R/mllib.R

+#'
+#' # save fitted model to input path
+#' path <- "path/to/model"
+#' write.ml(model, path)


hi @junyangq , isn't it write.ml (KM) you are talking about?

Yes, can you modify the doc of write.ml method for KM to bring it to the same page as other KM methods?

oh sorry I see what you mean now, you mean bring it to the Usage section of this page, right? I'll try to do it

keypointt · 2016-06-27T22:20:58Z

I just updated the screenshot in PR description

SparkQA · 2016-06-27T22:52:30Z

Test build #61328 has finished for PR 13921 at commit 6a67a49.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

junyangq · 2016-06-27T22:52:51Z

That looks better now. Two more comments

the description section can be made more compact as in [SPARK-16142] [R] group naiveBayes method docs in a single Rd #13877
duplicated arguments should be deleted as well

keypointt · 2016-06-27T23:24:50Z

could you please be more specific on compact? like what should be changed then? I'm not getting it completely

I'm removing the duplicate arguments now

mengxr · 2016-06-27T23:55:37Z

R/pkg/R/mllib.R

@@ -266,9 +266,9 @@ setMethod("summary", signature(object = "NaiveBayesModel"),
            return(list(apriori = apriori, tables = tables))
          })

-#' Fit a k-means model
+#' K-Means Model


K-Means Clustering Model

sorry I actually meant the description could be more like plain sentences, rather than bullet points.

This appears as the title, like in Generalized Linear Models.

mengxr · 2016-06-27T23:56:36Z

@keypointt For quicker tests, you can run R/create-docs.sh and then check the html doc under R/pkg/html. It would be much faster than jekyll build.

SparkQA · 2016-06-28T00:14:12Z

Test build #61331 has finished for PR 13921 at commit 1f4ef90.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-06-28T00:17:03Z

Test build #61332 has finished for PR 13921 at commit aeface5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

keypointt · 2016-06-28T03:40:50Z

@junyangq oh I got it thanks
@mengxr I just realized the jekyll build is just copying over whatever under R/pkg/html/, thanks for the tip

mengxr · 2016-06-28T03:42:32Z

I just merged #13927. So please fetch and merge the latest master if you want to push an update to avoid merge conflict (hopefully none).

SparkQA · 2016-06-28T03:43:52Z

Test build #61347 has finished for PR 13921 at commit 426c659.

This patch fails some tests.
This patch merges cleanly.
This patch adds no public classes.

keypointt · 2016-06-28T03:47:13Z

OK doing it now

SparkQA · 2016-06-28T03:53:19Z

Test build #61348 has finished for PR 13921 at commit 70c312f.

This patch fails some tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class ShowFunctionsCommand(

mengxr · 2016-06-28T05:58:25Z

I think the error was because this PR left predict, write.ml, etc documented without title. So this PR has to be combined with SPARK-16144. Basically, let us add some doc to the function declarations under generics.R.

cc: @yinxusen

keypointt · 2016-06-28T07:09:56Z

hi @mengxr I tried to modifiy in generics.R

#' @rdname spark.kmeans
#' @export
setGeneric("spark.kmeans", function(data, formula, ...) { standardGeneric("spark.kmeans") })

tried many different ways but still not working, since I'm not familiar with R docs, maybe here I need your help so that it's not blocking the other tickets.

How to add Sections \title, and \name? I guess it's because both algorithms are trying to define a predict or write.ml

yinxusen · 2016-06-28T07:43:21Z

@keypointt You need to add titles for predict and write.ml. Like the first line below.

#' This is a title for write.ml
#' @rdname write.ml
#' @export
setGeneric("write.ml", function(object, path, ...) { standardGeneric("write.ml") })

keypointt · 2016-06-28T16:37:27Z

oh I see, thanks a lot

SparkQA · 2016-06-28T17:18:14Z

Test build #61388 has finished for PR 13921 at commit 6fa1b60.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

felixcheung · 2016-06-28T18:46:50Z

Actually don't do this?

#' This is a title for write.ml
#' @rdname write.ml
#' @export
setGeneric("write.ml", function(object, path, ...) { standardGeneric("write.ml") })

it will create a dummy page with no content for write.ml. And generally we don't put more than @Rdname and @export in generics.R - it is harder to manage from there.

I also was thinking about the discoverability of "write.ml", "predict" and so on in the doc. I think it is better to have full page for "write.ml" and then link to write.ml(KM), write.ml(GLM) etc from there.

keypointt · 2016-06-28T18:50:07Z

I vote for dis-coverability of "write.ml", "predict"

felixcheung · 2016-06-28T18:50:25Z

And to elaborate, the reason why it is complaining is because the line in generics.R is the only reference to @Rdname write.ml. To fix, you could either change that to @Rdname spark.kmeans (which doesn't make sense because there are multiple implementation of write.ml for different model), or have an actual @Rdname write.ml block in mllib.R - this also helps to redirect to pages on other mllib models in the event someone is looking for write.ml and doesn't know to look in spark.kmeans for write.ml.

keypointt · 2016-06-28T19:49:54Z

if we are to have an actual @Rdname write.ml block in mllib.R, then it has to be enforced to all documentation stories under https://issues.apache.org/jira/browse/SPARK-16090, to remove individual write.ml and predict of each model

I just notice that read.ml has its own page, but not write.ml, to make both consistent, more work is needed here. Below is what it is like for current code.

write.ml

read.ml

mengxr · 2016-06-28T20:09:27Z

@keypointt Could you add seealso links to the generic predict and write.ml doc?

@felixcheung I think it is useful to have a page for the generic predict and write.ml. We just need to add some links to them so they do provide some information. Btw, is it necessary to define predict under generics.R? Is it a common practice in R packages?

keypointt · 2016-06-28T20:25:26Z

sure, I tried

#' @seealso \link{predict}, \link{read.ml}, \link{write.ml}

and it's like below for spark.kmeans page, is this what you mean?

But if I remove the previously added title in generics.R, then errors still pop up

SparkQA · 2016-06-28T21:36:48Z

Test build #61410 has finished for PR 13921 at commit 402026c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

mengxr · 2016-06-29T18:25:46Z

The grouping of k-means methods LGTM. So I'm merging this into master and branch-2.0 and leave the remaining issues to SPARK-16144. Thanks!

…R doc https://issues.apache.org/jira/browse/SPARK-16140 ## What changes were proposed in this pull request? Group the R doc of spark.kmeans, predict(KM), summary(KM), read/write.ml(KM) under Rd spark.kmeans. The example code was updated. ## How was this patch tested? Tested on my local machine And on my laptop `jekyll build` is failing to build API docs, so here I can only show you the html I manually generated from Rd files, with no CSS applied, but the doc content should be there. ![screenshotkmeans](https://cloud.githubusercontent.com/assets/3925641/16403203/c2c9ca1e-3ca7-11e6-9e29-f2164aee75fc.png) Author: Xin Ren <iamshrek@126.com> Closes #13921 from keypointt/SPARK-16140. (cherry picked from commit 8c9cd0a) Signed-off-by: Xiangrui Meng <meng@databricks.com>

## What changes were proposed in this pull request? From SPARK-16140/PR #13921 - the issue is we left write.ml doc empty: ![image](https://cloud.githubusercontent.com/assets/8969467/16481934/856dd0ea-3e62-11e6-9474-e4d57d1ca001.png) Here's what I meant as the fix: ![image](https://cloud.githubusercontent.com/assets/8969467/16481943/911f02ec-3e62-11e6-9d68-17363a9f5628.png) ![image](https://cloud.githubusercontent.com/assets/8969467/16481950/9bc057aa-3e62-11e6-8127-54870701c4b1.png) I didn't realize there was already a JIRA on this. mengxr yanboliang ## How was this patch tested? check doc generated. Author: Felix Cheung <felixcheung_m@hotmail.com> Closes #13993 from felixcheung/rmllibdoc. (cherry picked from commit 7f38b9d) Signed-off-by: Shivaram Venkataraman <shivaram@cs.berkeley.edu>

## What changes were proposed in this pull request? From SPARK-16140/PR #13921 - the issue is we left write.ml doc empty: ![image](https://cloud.githubusercontent.com/assets/8969467/16481934/856dd0ea-3e62-11e6-9474-e4d57d1ca001.png) Here's what I meant as the fix: ![image](https://cloud.githubusercontent.com/assets/8969467/16481943/911f02ec-3e62-11e6-9d68-17363a9f5628.png) ![image](https://cloud.githubusercontent.com/assets/8969467/16481950/9bc057aa-3e62-11e6-8127-54870701c4b1.png) I didn't realize there was already a JIRA on this. mengxr yanboliang ## How was this patch tested? check doc generated. Author: Felix Cheung <felixcheung_m@hotmail.com> Closes #13993 from felixcheung/rmllibdoc.

keypointt added 2 commits June 27, 2016 00:22

[SPARK-16140] group k-means method in generated doc

3fee765

[SPARK-16140] minor changes

1228483

keypointt reviewed Jun 27, 2016
View reviewed changes

[SPARK-16140] bring individual methods to the same spark.kmeans page

6a67a49

[SPARK-16140] remove duplicated arguments 'object'

aeface5

keypointt force-pushed the SPARK-16140 branch from 1f4ef90 to aeface5 Compare June 27, 2016 23:44

mengxr reviewed Jun 27, 2016
View reviewed changes

[SPARK-16140] more changes on style

426c659

Merge branch 'master' into SPARK-16140

70c312f

[SPARK-16140] add title for 'predict' and 'write.ml' generics

6fa1b60

[SPARK-16140] add link to generic predict and write.ml doc

402026c

asfgit closed this in 8c9cd0a Jun 29, 2016

felixcheung mentioned this pull request Jun 30, 2016

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

Closed

[SPARK-16140][MLlib][SparkR][Docs] Group k-means method in generated R doc #13921

[SPARK-16140][MLlib][SparkR][Docs] Group k-means method in generated R doc #13921

Uh oh!

Conversation

keypointt commented Jun 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

SparkQA commented Jun 27, 2016

Uh oh!

mengxr commented Jun 27, 2016

Uh oh!

junyangq commented Jun 27, 2016

Uh oh!

keypointt Jun 27, 2016

Choose a reason for hiding this comment

Uh oh!

junyangq Jun 27, 2016

Choose a reason for hiding this comment

Uh oh!

keypointt Jun 27, 2016

Choose a reason for hiding this comment

Uh oh!

keypointt commented Jun 27, 2016

Uh oh!

SparkQA commented Jun 27, 2016

Uh oh!

junyangq commented Jun 27, 2016

Uh oh!

keypointt commented Jun 27, 2016

Uh oh!

mengxr Jun 27, 2016

Choose a reason for hiding this comment

Uh oh!

junyangq Jun 28, 2016

Choose a reason for hiding this comment

Uh oh!

mengxr Jun 28, 2016

Choose a reason for hiding this comment

Uh oh!

mengxr commented Jun 27, 2016

Uh oh!

SparkQA commented Jun 28, 2016

Uh oh!

SparkQA commented Jun 28, 2016

Uh oh!

keypointt commented Jun 28, 2016

Uh oh!

mengxr commented Jun 28, 2016

Uh oh!

SparkQA commented Jun 28, 2016

Uh oh!

keypointt commented Jun 28, 2016

Uh oh!

SparkQA commented Jun 28, 2016

Uh oh!

mengxr commented Jun 28, 2016

Uh oh!

keypointt commented Jun 28, 2016

Uh oh!

yinxusen commented Jun 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keypointt commented Jun 28, 2016

Uh oh!

SparkQA commented Jun 28, 2016

Uh oh!

felixcheung commented Jun 28, 2016

Uh oh!

keypointt commented Jun 28, 2016

Uh oh!

felixcheung commented Jun 28, 2016

Uh oh!

keypointt commented Jun 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mengxr commented Jun 28, 2016

Uh oh!

keypointt commented Jun 28, 2016

keypointt commented Jun 27, 2016 •

edited

Loading

yinxusen commented Jun 28, 2016 •

edited

Loading

keypointt commented Jun 28, 2016 •

edited

Loading