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

[SPARK-7482][SparkR] Rename some DataFrame API methods in SparkR to match their counterparts in Scala. #6007

Closed
wants to merge 3 commits into from
Closed

[SPARK-7482][SparkR] Rename some DataFrame API methods in SparkR to match their counterparts in Scala. #6007

wants to merge 3 commits into from

Conversation

sun-rui
Copy link
Contributor

@sun-rui sun-rui commented May 8, 2015

No description provided.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented May 8, 2015

Test build #32225 has started for PR 6007 at commit e861dc1.

@SparkQA
Copy link

SparkQA commented May 8, 2015

Test build #32225 has finished for PR 6007 at commit e861dc1.

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

@AmplabJenkins
Copy link

Merged build finished. Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32225/
Test FAILed.

@shivaram
Copy link
Contributor

shivaram commented May 8, 2015

@sun-rui - Thanks for the PR. However I am having second thoughts about some of the renames. For some functions like filter and I guess to a certain extent sample its probably fine to overload as they are less often used functions.

However load and save are probably more widely used functions and our functions actually do something different from the load and save in R. One idea might be to rename these to read or read.df as these functions typically load data from external formats (like read.csv or read.table etc.). Any thoughts ?

cc @rxin

@sun-rui
Copy link
Contributor Author

sun-rui commented May 9, 2015

@shivaram, I think it is OK to rename APIs to ones that R user are accustomed to, for example, read.df and write.df. It would be better if there are more feedback from R community.

@shivaram
Copy link
Contributor

shivaram commented May 9, 2015

Yeah lets go with read.df and write.dffor now - We can also just alias these to loadDF and saveDF. We should revisit this when we graduate SparkR from experimental status and see if we have any user feedback

@AmplabJenkins
Copy link

Build triggered.

@AmplabJenkins
Copy link

Build started.

@SparkQA
Copy link

SparkQA commented May 12, 2015

Test build #32465 has started for PR 6007 at commit 9967c0f.

@SparkQA
Copy link

SparkQA commented May 12, 2015

Test build #32465 has finished for PR 6007 at commit 9967c0f.

  • This patch fails Spark unit tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Build finished. Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32465/
Test FAILed.

@shivaram
Copy link
Contributor

@sun-rui Could you bring this up to date with master ?

@shivaram
Copy link
Contributor

Jenkins, retest this please

@AmplabJenkins
Copy link

Build triggered.

@AmplabJenkins
Copy link

Build started.

@SparkQA
Copy link

SparkQA commented May 12, 2015

Test build #32506 has started for PR 6007 at commit 9967c0f.

shivaram
shivaram reviewed May 12, 2015
View reviewed changes
R/pkg/R/SQLContext.R
#' }

loadDF <- function(sqlCtx, path = NULL, source = NULL, ...) {
read.df <- loadDF <- function(sqlCtx, path = NULL, source = NULL, ...) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't know this was a valid way to declare alias functions in R. Hmm - But it seems to go against the style in the rest of the file. Could we just declare a new function read.df similar to other aliases ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@SparkQA
Copy link

SparkQA commented May 12, 2015

Test build #32506 has finished for PR 6007 at commit 9967c0f.

  • This patch passes all tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Build finished. Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32506/
Test PASSed.

@sun-rui
Copy link
Contributor Author

sun-rui commented May 13, 2015

rebased to master.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented May 13, 2015

Test build #32582 has started for PR 6007 at commit 5c5cf5e.

@shivaram
Copy link
Contributor

Thanks for the update. LGTM. Will wait for Jenkins and then merge

@SparkQA
Copy link

SparkQA commented May 13, 2015

Test build #32582 has finished for PR 6007 at commit 5c5cf5e.

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

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32582/
Test PASSed.

asfgit pushed a commit that referenced this pull request May 13, 2015
@shivaram
[SPARK-7482] [SPARKR] Rename some DataFrame API methods in SparkR to …
b18f1c6 
…match their counterparts in Scala.

Author: Sun Rui <rui.sun@intel.com>

Closes #6007 from sun-rui/SPARK-7482 and squashes the following commits:

5c5cf5e [Sun Rui] Implement alias loadDF() as a new function.
3a30c10 [Sun Rui] Rename load()/save() to read.df()/write.df(). Also add loadDF()/saveDF() as aliases.
9f569d6 [Sun Rui] [SPARK-7482][SparkR] Rename some DataFrame API methods in SparkR to match their counterparts in Scala.

(cherry picked from commit df9b94a)
Signed-off-by: Shivaram Venkataraman <shivaram@cs.berkeley.edu>
@asfgit asfgit closed this in df9b94a May 13, 2015
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request May 28, 2015
@jeanlyn
[SPARK-7482] [SPARKR] Rename some DataFrame API methods in SparkR to …
92572d8 
…match their counterparts in Scala.

Author: Sun Rui <rui.sun@intel.com>

Closes apache#6007 from sun-rui/SPARK-7482 and squashes the following commits:

5c5cf5e [Sun Rui] Implement alias loadDF() as a new function.
3a30c10 [Sun Rui] Rename load()/save() to read.df()/write.df(). Also add loadDF()/saveDF() as aliases.
9f569d6 [Sun Rui] [SPARK-7482][SparkR] Rename some DataFrame API methods in SparkR to match their counterparts in Scala.
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request Jun 12, 2015
@jeanlyn
[SPARK-7482] [SPARKR] Rename some DataFrame API methods in SparkR to …
aa1139f 
…match their counterparts in Scala.

Author: Sun Rui <rui.sun@intel.com>

Closes apache#6007 from sun-rui/SPARK-7482 and squashes the following commits:

5c5cf5e [Sun Rui] Implement alias loadDF() as a new function.
3a30c10 [Sun Rui] Rename load()/save() to read.df()/write.df(). Also add loadDF()/saveDF() as aliases.
9f569d6 [Sun Rui] [SPARK-7482][SparkR] Rename some DataFrame API methods in SparkR to match their counterparts in Scala.
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
@nemccarthy
[SPARK-7482] [SPARKR] Rename some DataFrame API methods in SparkR to …
14f5f34 
…match their counterparts in Scala.

Author: Sun Rui <rui.sun@intel.com>

Closes apache#6007 from sun-rui/SPARK-7482 and squashes the following commits:

5c5cf5e [Sun Rui] Implement alias loadDF() as a new function.
3a30c10 [Sun Rui] Rename load()/save() to read.df()/write.df(). Also add loadDF()/saveDF() as aliases.
9f569d6 [Sun Rui] [SPARK-7482][SparkR] Rename some DataFrame API methods in SparkR to match their counterparts in Scala.
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
4 participants
@sun-rui @AmplabJenkins @SparkQA @shivaram