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-7435[R]: Make DataFrame.show() consistent with that of Scala and pySpark #5989

Closed
wants to merge 3 commits into from
Closed

Spark-7435[R]: Make DataFrame.show() consistent with that of Scala and pySpark #5989

wants to merge 3 commits into from

Conversation

rekhajoshm
Copy link
Contributor

No description provided.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@shivaram
Copy link
Contributor

shivaram commented May 7, 2015

Thanks @rekhajoshm -- I actually think we can remove showDF as I described in https://issues.apache.org/jira/browse/SPARK-7435 - Lets discuss about this with @sun-rui as well on the JIRA and then come to this PR.

@sun-rui
Copy link
Contributor

sun-rui commented May 8, 2015

@rekhajoshm , Thank you. As discussed in the JIRA issue, we can keep these names as is. But we can improve the implementation of showDF() to print c-style strings.
str <- callJMethod(x@sdf, "showString", numToInt(numRows))
cat(str)
Also you need to update the test case for showDF() accordingly. capture.output() can be used to capture the result of showDF() for test.
Could you update the PR?

@rekhajoshm
Copy link
Contributor Author

Thank you @shivaram and @sun-rui for quick reply and good discussion.Updated patch for review comments.thanks

@shivaram
Copy link
Contributor

shivaram commented May 8, 2015

Jenkins, ok to test

shivaram
shivaram reviewed May 8, 2015
View reviewed changes
R/pkg/R/DataFrame.R
@@ -184,7 +185,7 @@ setMethod("showDF",
#' sqlCtx <- sparkRSQL.init(sc)
#' path <- "path/to/file.json"
#' df <- jsonFile(sqlCtx, path)
#' show(df)
#' show
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit: This should be df and not show

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@shivaram
Copy link
Contributor

shivaram commented May 8, 2015

Thanks @rekhajoshm - Change mostly looks good to me. I had a minor comment inline

@SparkQA
Copy link

SparkQA commented May 8, 2015

Test build #32280 has started for PR 5989 at commit cfc9e02.

@shivaram
Copy link
Contributor

shivaram commented May 8, 2015

@srowen Could you help add @rekhajoshm as developer in the Spark JIRA so this issue can be assigned ?

@SparkQA
Copy link

SparkQA commented May 9, 2015

Test build #32280 has finished for PR 5989 at commit cfc9e02.

  • 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/32280/
Test PASSed.

@shivaram
Copy link
Contributor

The comment fix is actually pretty minor - I can fix it up during the merge. Thanks @rekhajoshm for the PR. Merging this to master and branch-1.4

asfgit pushed a commit that referenced this pull request May 12, 2015
@rekhajoshm @shivaram
[SPARK-7435] [SPARKR] Make DataFrame.show() consistent with that of S…
835a770 
…cala and pySpark

Author: Joshi <[email protected]>
Author: Rekha Joshi <[email protected]>

Closes #5989 from rekhajoshm/fix/SPARK-7435 and squashes the following commits:

cfc9e02 [Joshi] Spark-7435[R]: updated patch for review comments
62becc1 [Joshi] SPARK-7435: Update to DataFrame
e3677c9 [Rekha Joshi] Merge pull request #1 from apache/master

(cherry picked from commit b94a933)
Signed-off-by: Shivaram Venkataraman <[email protected]>
@asfgit asfgit closed this in b94a933 May 12, 2015
@rekhajoshm
Copy link
Contributor Author

Thanks @shivaram

jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request May 28, 2015
@rekhajoshm @jeanlyn
[SPARK-7435] [SPARKR] Make DataFrame.show() consistent with that of S…
6630093 
…cala and pySpark

Author: Joshi <[email protected]>
Author: Rekha Joshi <[email protected]>

Closes apache#5989 from rekhajoshm/fix/SPARK-7435 and squashes the following commits:

cfc9e02 [Joshi] Spark-7435[R]: updated patch for review comments
62becc1 [Joshi] SPARK-7435: Update to DataFrame
e3677c9 [Rekha Joshi] Merge pull request apache#1 from apache/master
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request Jun 12, 2015
@rekhajoshm @jeanlyn
[SPARK-7435] [SPARKR] Make DataFrame.show() consistent with that of S…
4ed131a 
…cala and pySpark

Author: Joshi <[email protected]>
Author: Rekha Joshi <[email protected]>

Closes apache#5989 from rekhajoshm/fix/SPARK-7435 and squashes the following commits:

cfc9e02 [Joshi] Spark-7435[R]: updated patch for review comments
62becc1 [Joshi] SPARK-7435: Update to DataFrame
e3677c9 [Rekha Joshi] Merge pull request apache#1 from apache/master
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
@rekhajoshm @nemccarthy
[SPARK-7435] [SPARKR] Make DataFrame.show() consistent with that of S…
50c1e5a 
…cala and pySpark

Author: Joshi <[email protected]>
Author: Rekha Joshi <[email protected]>

Closes apache#5989 from rekhajoshm/fix/SPARK-7435 and squashes the following commits:

cfc9e02 [Joshi] Spark-7435[R]: updated patch for review comments
62becc1 [Joshi] SPARK-7435: Update to DataFrame
e3677c9 [Rekha Joshi] Merge pull request apache#1 from apache/master
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.
5 participants
@rekhajoshm @AmplabJenkins @shivaram @sun-rui @SparkQA