Skip to content

Conversation

@actuaryzhang
Copy link
Contributor

What changes were proposed in this pull request?

Some examples in the DataFrame methods are syntactically wrong, even though they are pseudo code. Fix these and some style issues.

@actuaryzhang
Copy link
Contributor Author

@felixcheung

@SparkQA
Copy link

SparkQA commented May 16, 2017

Test build #76976 has finished for PR 18003 at commit d2f4d29.

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

#' @param object a SparkDataFrame
#' @examples \dontrun{
#' @examples
#' \dontrun{
Copy link
Contributor

Choose a reason for hiding this comment

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

There are a lot of these in functions.R. Maybe you could fix them too with this PR?

@actuaryzhang
Copy link
Contributor Author

@bdwyer2 Thanks for the suggestion. I ran a check on all the R scripts and fixed the style issues.

@actuaryzhang actuaryzhang changed the title [SparkR] Fix bad examples in DataFrame methods [SparkR] Fix bad examples in DataFrame methods and style issues May 16, 2017
@SparkQA
Copy link

SparkQA commented May 16, 2017

Test build #76979 has finished for PR 18003 at commit a919d9f.

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

#' df <- read.json(path)
#' write.df(df, "myfile", "parquet", "overwrite")
#' saveDF(df, parquetPath2, "parquet", mode = saveMode, mergeSchema = mergeSchema)
#' saveDF(df, parquetPath2, "parquet", mode = "overwrite")
Copy link
Member

Choose a reason for hiding this comment

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

the example with these parameter are actually intentional - it is to show what additional properties can be set.
but I'd agree if they run as-is it would be preferred

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to keep the mergeSchema param though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added it back

#' @export
#' @examples \dontrun{abs(df$c)}
#' @examples
#' \dontrun{abs(df$c)}
Copy link
Member

Choose a reason for hiding this comment

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

I'm actually not sure about this change - if the example is simple and is single line, I think they can go on the same line as @example tag.
More importantly though, I don't think majority of SQL function examples like this is usable or useful actually, and would rather have multi-line "real" example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. Reverted the change.

Wayne Zhang added 2 commits May 16, 2017 16:43
@actuaryzhang
Copy link
Contributor Author

@felixcheung Thanks for your suggestion. I reverted back the change on single line example.
I agree that the examples in sql are not informative. I will put in better examples in another PR.

@SparkQA
Copy link

SparkQA commented May 17, 2017

Test build #76993 has finished for PR 18003 at commit 1450dd1.

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

Copy link
Member

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

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

cool, thanks, LGTM.

@felixcheung
Copy link
Member

felixcheung commented May 17, 2017

FYI. Can't merge at the moment - give me a day or 2.

@actuaryzhang
Copy link
Contributor Author

Thanks for the update. I will work on better examples on these functions.

@asfgit asfgit closed this in 7f203a2 May 19, 2017
@felixcheung
Copy link
Member

merged to master

@actuaryzhang actuaryzhang deleted the sparkRDoc3 branch May 19, 2017 19:51
lycplus pushed a commit to lycplus/spark that referenced this pull request May 24, 2017
## What changes were proposed in this pull request?
Some examples in the DataFrame methods are syntactically wrong, even though they are pseudo code. Fix these and some style issues.

Author: Wayne Zhang <actuaryzhang@uber.com>

Closes apache#18003 from actuaryzhang/sparkRDoc3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants