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

Jump to bottom

[SPARKR][SPARK-16507] Add a CRAN checker, fix Rd aliases #14173

Closed
wants to merge 8 commits into from
Closed

[SPARKR][SPARK-16507] Add a CRAN checker, fix Rd aliases #14173

wants to merge 8 commits into from

Conversation

shivaram
Copy link
Contributor

What changes were proposed in this pull request?

Add a check-cran.sh script that runs R CMD check as CRAN. Also fixes a number of issues pointed out by the check. These include

How was this patch tested?

SparkR unit tests, running the above mentioned script

shivaram added 2 commits July 11, 2016 18:41
@shivaram
Add a CRAN checker.
be7a74f 
Also update Rd documentation to address a number of CRAN check
warnings.
@shivaram
Use x instead of col in window functions
62be1cb
@shivaram
Copy link
Contributor Author

cc @felixcheung @sun-rui @mengxr @junyangq

@SparkQA
Copy link

SparkQA commented Jul 13, 2016

Test build #62215 has finished for PR 14173 at commit 62be1cb.

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

@SparkQA
Copy link

SparkQA commented Jul 13, 2016

Test build #62218 has finished for PR 14173 at commit 3818d1c.

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

felixcheung
felixcheung reviewed Jul 13, 2016
View reviewed changes
R/pkg/R/SQLContext.R
@@ -267,6 +267,10 @@ as.DataFrame.default <- function(data, schema = NULL, samplingRatio = 1.0) {
createDataFrame(data, schema, samplingRatio)
}

#' @rdname createDataFrame
#' @aliases createDataFrame
Copy link
Member

@felixcheung felixcheung Jul 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

should aliases here be as.DataFrame so it could be found via ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch - fixed

@SparkQA
Copy link

SparkQA commented Jul 13, 2016

Test build #62279 has finished for PR 14173 at commit a2275ae.

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

@shivaram
Copy link
Contributor Author

@felixcheung Can you check if you see the same error as Jenkins on your machine ? On my machine the install and tests seem to pass, so I think this is a R / roxygen / devtools version problem.

@SparkQA
Copy link

SparkQA commented Jul 13, 2016

Test build #62283 has finished for PR 14173 at commit 6c9309e.

  • This patch fails RAT tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@felixcheung
Copy link
Member

I ran both knitr and lintr on this, both succeeded.

@shivaram
Copy link
Contributor Author

I managed to reproduce the issue that Jenkins was hitting. It had to do with using @method on a as.DataFrame that was creating an error on html generation. I just removed that and it seems to be passing now.

@felixcheung
Copy link
Member

right, that line would not be necessary.

@SparkQA
Copy link

SparkQA commented Jul 14, 2016

Test build #62284 has finished for PR 14173 at commit 3299242.

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

@felixcheung
Copy link
Member

LGTM

@SparkQA
Copy link

SparkQA commented Jul 14, 2016

Test build #62336 has finished for PR 14173 at commit bc9305d.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jul 14, 2016

Test build #62335 has finished for PR 14173 at commit e4ad2ec.

  • This patch fails SparkR unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@shivaram
Copy link
Contributor Author

Jenkins, retest this please

@SparkQA
Copy link

SparkQA commented Jul 14, 2016

Test build #62348 has finished for PR 14173 at commit bc9305d.

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

@shivaram
Copy link
Contributor Author

@felixcheung Just to confirm, based on discussion in SPARK-16508 is this change good to merge ?

@felixcheung
Copy link
Member

Yap - I'll check if there's anything left as per SPARK-16508

@shivaram
Copy link
Contributor Author

Cool. Merging this to master, branch-2.0

asfgit pushed a commit that referenced this pull request Jul 17, 2016
@shivaram
[SPARK-16507][SPARKR] Add a CRAN checker, fix Rd aliases
c527e9e 
## What changes were proposed in this pull request?

Add a check-cran.sh script that runs `R CMD check` as CRAN. Also fixes a number of issues pointed out by the check. These include
- Updating `DESCRIPTION` to be appropriate
- Adding a .Rbuildignore to ignore lintr, src-native, html that are non-standard files / dirs
- Adding aliases to all S4 methods in DataFrame, Column, GroupedData etc.  This is required as stated in https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Documenting-S4-classes-and-methods
- Other minor fixes

## How was this patch tested?

SparkR unit tests, running the above mentioned script

Author: Shivaram Venkataraman <shivaram@cs.berkeley.edu>

Closes #14173 from shivaram/sparkr-cran-changes.

(cherry picked from commit c33e4b0)
Signed-off-by: Shivaram Venkataraman <shivaram@cs.berkeley.edu>
@asfgit asfgit closed this in c33e4b0 Jul 17, 2016
@junyangq junyangq mentioned this pull request Jul 19, 2016
Closed
asfgit pushed a commit that referenced this pull request Aug 10, 2016
@junyangq @shivaram
[SPARK-16579][SPARKR] add install.spark function
214ba66 
## What changes were proposed in this pull request?

Add an install_spark function to the SparkR package. User can run `install_spark()` to install Spark to a local directory within R.

Updates:

Several changes have been made:

- `install.spark()`
    - check existence of tar file in the cache folder, and download only if not found
    - trial priority of mirror_url look-up: user-provided -> preferred mirror site from apache website -> hardcoded backup option
    - use 2.0.0

- `sparkR.session()`
    - can install spark when not found in `SPARK_HOME`

## How was this patch tested?

Manual tests, running the check-cran.sh script added in #14173.

Author: Junyang Qian <junyangq@databricks.com>

Closes #14258 from junyangq/SPARK-16579.
asfgit pushed a commit that referenced this pull request Aug 10, 2016
@junyangq @shivaram
[SPARK-16579][SPARKR] add install.spark function
d3a30d2 
Add an install_spark function to the SparkR package. User can run `install_spark()` to install Spark to a local directory within R.

Updates:

Several changes have been made:

- `install.spark()`
    - check existence of tar file in the cache folder, and download only if not found
    - trial priority of mirror_url look-up: user-provided -> preferred mirror site from apache website -> hardcoded backup option
    - use 2.0.0

- `sparkR.session()`
    - can install spark when not found in `SPARK_HOME`

Manual tests, running the check-cran.sh script added in #14173.

Author: Junyang Qian <junyangq@databricks.com>

Closes #14258 from junyangq/SPARK-16579.

(cherry picked from commit 214ba66)
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
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@shivaram @SparkQA @felixcheung