Skip to content

Conversation

@falaki
Copy link
Contributor

@falaki falaki commented Sep 8, 2016

What changes were proposed in this pull request?

additional options were not passed down in write.df. This patch fixes that

How was this patch tested?

Adds unit test for passing header = "true" for CSV data source in R unit tests

@SparkQA
Copy link

SparkQA commented Sep 8, 2016

Test build #65065 has finished for PR 15007 at commit 7da87cd.

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

write <- callJMethod(write, "format", source)
write <- callJMethod(write, "mode", jmode)
write <- callJMethod(write, "save", path)
write <- callJMethod(write, "save", options)
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure I see a save method which takes in options. What if we just call write <- callJMethod(write, "options", options) in the line before write <- callJMethod(write, "save", path) ?

Copy link
Member

Choose a reason for hiding this comment

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

oops, sorry I broke it.
right, I think the right approach is

write <- callJMethod(write, "options", options)
write <- callJMethod(write, "save", path)

save should be the last call.

@SparkQA
Copy link

SparkQA commented Sep 8, 2016

Test build #65067 has finished for PR 15007 at commit 78d1dce.

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

csvPath2 <- tempfile(pattern = "csvtest2", fileext = ".csv")
write.df(df2, path = csvPath2, "csv", header = "true")
df3 <- read.df(csvPath2, "csv", header = "true")
exect_equal(nrow(df3), nrow(df2))
Copy link
Member

Choose a reason for hiding this comment

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

typo expect_equal

@felixcheung
Copy link
Member

sorry for the trouble, I opened #15010 with the fix

@SparkQA
Copy link

SparkQA commented Sep 8, 2016

Test build #65078 has finished for PR 15007 at commit e2f88dc.

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

@shivaram
Copy link
Contributor

shivaram commented Sep 8, 2016

@falaki We can use the PR at #15010 and close this one ?

@falaki falaki closed this Sep 8, 2016
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