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.

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

[SPARK-11756][SPARKR] Fix use of aliases - SparkR can not output help information for SparkR:::summary correctly #9750

Closed
wants to merge 2 commits into from

Conversation

felixcheung
Copy link
Member

Fix use of aliases and changes uses of @rdname and @Seealso
@aliases is the hint for ? - it should not be linked to some other name - those should be @Seealso
https://cran.r-project.org/web/packages/roxygen2/vignettes/rd.html

Clean up usage on @family, as multiple use of @family with the same @rdname is causing duplicated See Also html blocks (like http://spark.apache.org/docs/latest/api/R/count.html)
Also changing some @rdname for dplyr-like variant for better R user visibility in R doc, eg. rbind, summary, mutate, summarize

@shivaram @yanboliang

@SparkQA
Copy link

SparkQA commented Nov 17, 2015

Test build #46034 has finished for PR 9750 at commit 474d6af.

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

@shivaram
Copy link
Contributor

cc @sun-rui

@felixcheung
Copy link
Member Author

this PR is documentation only, no code change here

@felixcheung
Copy link
Member Author

@yu-iskw would love to have your feedback

@felixcheung
Copy link
Member Author

any more comment?

@shivaram
Copy link
Contributor

@yanboliang Could you help us review this PR by trying it out and seeing if it fixes the issue ?

@yanboliang
Copy link
Contributor

Sorry for late response. It can fix my issue mentioned at JIRA. But I also found new issues that the generated doc of describe will include the summary method for signature PipelineModel which may not make sense. Here I list the USAGE section of describe doc:

describe(x, col, ...)

summary(object, ...)

## S4 method for signature 'DataFrame,character'
describe(x, col, ...)

## S4 method for signature 'DataFrame,ANY'
describe(x)

## S4 method for signature 'DataFrame'
summary(object, ...)

## S4 method for signature 'PipelineModel'
summary(object, ...)

@felixcheung
Copy link
Member Author

It actually is a common practice to include function for types on the same help page of the function name.
As you can see https://stat.ethz.ch/R-manual/R-devel/library/base/html/summary.html
It has summary of data.frame, matrix, factor, lm, glm, on the same page (deeper links for lm & glm)
Since they all have the same name 'summary' - shouldn't they be? Do you think they are semantically very different?

@yanboliang
Copy link
Contributor

OK, got it. LGTM for me. Thanks for your PR.

@@ -254,7 +254,6 @@ setMethod("dtypes",
#' @family DataFrame functions
#' @rdname columns
#' @name columns
#' @aliases names
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldnt we add @seealso names here ?

Copy link
Member Author

Choose a reason for hiding this comment

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

name is on the same doc page as column (maybe we should change @rdname to names for R friendliness)

#' @rdname columns
 #' @name names
 setMethod("names",

@shivaram
Copy link
Contributor

Thanks @felixcheung -- LGTM. Had some minor inline comments about adding seealso in places where we removed aliases

@shivaram
Copy link
Contributor

Merging this to master and branch-1.6

@asfgit asfgit closed this in a6239d5 Nov 20, 2015
asfgit pushed a commit that referenced this pull request Nov 20, 2015
… information for SparkR:::summary correctly

Fix use of aliases and changes uses of rdname and seealso
`aliases` is the hint for `?` - it should not be linked to some other name - those should be seealso
https://cran.r-project.org/web/packages/roxygen2/vignettes/rd.html

Clean up usage on family, as multiple use of family with the same rdname is causing duplicated See Also html blocks (like http://spark.apache.org/docs/latest/api/R/count.html)
Also changing some rdname for dplyr-like variant for better R user visibility in R doc, eg. rbind, summary, mutate, summarize

shivaram yanboliang

Author: felixcheung <felixcheung_m@hotmail.com>

Closes #9750 from felixcheung/rdocaliases.

(cherry picked from commit a6239d5)
Signed-off-by: Shivaram Venkataraman <shivaram@cs.berkeley.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants