You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/08/18 12:39:45 UTC
[GitHub] [arrow-cookbook] nealrichardson opened a new pull request #41: [R] Remove unnecessary head()s
nealrichardson opened a new pull request #41:
URL: https://github.com/apache/arrow-cookbook/pull/41
The data frames in question are only 3 lines long, so you don't need them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-cookbook] westonpace commented on a change in pull request #41: [R] Remove unnecessary head()s
Posted by GitBox <gi...@apache.org>.
westonpace commented on a change in pull request #41:
URL: https://github.com/apache/arrow-cookbook/pull/41#discussion_r691731369
##########
File path: r/content/reading_and_writing_data.Rmd
##########
@@ -60,11 +60,11 @@ Given a Parquet file, it can be read back in by using `arrow::read_parquet()`.
```{r, read_parquet}
parquet_tbl <- read_parquet("my_table.parquet")
-head(parquet_tbl)
+parquet_tbl
```
```{r, test_read_parquet, opts.label = "test"}
test_that("read_parquet works as expected", {
- expect_equivalent(dplyr::collect(parquet_tbl), tibble::tibble(group = c("A", "B", "C"), score = c(99, 97, 99)))
+ expect_identical(as.data.frame(parquet_tbl), tibble::tibble(group = c("A", "B", "C"), score = c(99, 97, 99)))
Review comment:
Looks like `expect_identical` doesn't pass here. I'm not exactly sure why. https://github.com/apache/arrow-cookbook/pull/41/checks?check_run_id=3361129104
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-cookbook] thisisnic commented on a change in pull request #41: [R] Remove unnecessary head()s
Posted by GitBox <gi...@apache.org>.
thisisnic commented on a change in pull request #41:
URL: https://github.com/apache/arrow-cookbook/pull/41#discussion_r691919239
##########
File path: r/content/reading_and_writing_data.Rmd
##########
@@ -60,11 +60,11 @@ Given a Parquet file, it can be read back in by using `arrow::read_parquet()`.
```{r, read_parquet}
parquet_tbl <- read_parquet("my_table.parquet")
-head(parquet_tbl)
+parquet_tbl
```
```{r, test_read_parquet, opts.label = "test"}
test_that("read_parquet works as expected", {
- expect_equivalent(dplyr::collect(parquet_tbl), tibble::tibble(group = c("A", "B", "C"), score = c(99, 97, 99)))
+ expect_identical(as.data.frame(parquet_tbl), tibble::tibble(group = c("A", "B", "C"), score = c(99, 97, 99)))
Review comment:
I think it's because `as.data.frame()` returns a `data.frame` object whereas `dplyr::collect()` returns a `tibble` object, so while the objects returned contain the same values, they are of different classes (which is something that `expect_identical` checks for).
Updating the expected return value to be created via `data.frame()` instead of `tibble::tibble()` would fix this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-cookbook] nealrichardson commented on a change in pull request #41: [R] Remove unnecessary head()s
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #41:
URL: https://github.com/apache/arrow-cookbook/pull/41#discussion_r692179176
##########
File path: r/content/reading_and_writing_data.Rmd
##########
@@ -60,11 +60,11 @@ Given a Parquet file, it can be read back in by using `arrow::read_parquet()`.
```{r, read_parquet}
parquet_tbl <- read_parquet("my_table.parquet")
-head(parquet_tbl)
+parquet_tbl
```
```{r, test_read_parquet, opts.label = "test"}
test_that("read_parquet works as expected", {
- expect_equivalent(dplyr::collect(parquet_tbl), tibble::tibble(group = c("A", "B", "C"), score = c(99, 97, 99)))
+ expect_identical(as.data.frame(parquet_tbl), tibble::tibble(group = c("A", "B", "C"), score = c(99, 97, 99)))
Review comment:
That's not what I see locally:
```
> my_table <- Table$create(tibble::tibble(group = c("A", "B", "C"), score = c(99, 97, 99)))
> write_parquet(my_table, "my_table.parquet")
> attributes(read_parquet("my_table.parquet"))
$names
[1] "group" "score"
$class
[1] "tbl_df" "tbl" "data.frame"
$row.names
[1] 1 2 3
> attributes(tibble::tibble(group = c("A", "B", "C"), score = c(99, 97, 99)))
$names
[1] "group" "score"
$row.names
[1] 1 2 3
$class
[1] "tbl_df" "tbl" "data.frame"
```
I see that the attributes are in a different order, which while not functionally relevant, could mean that they don't test equally. We just got a JIRA about this this week, in fact.
Note that `collect` just wraps `as.data.frame()`: https://github.com/apache/arrow/blob/master/r/R/dplyr-collect.R#L43-L49
Side note: if the cookbook tests were to use testthat 3rd edition, we'd get better diffs of what attributes don't match.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-cookbook] westonpace merged pull request #41: [R] Remove unnecessary head()s
Posted by GitBox <gi...@apache.org>.
westonpace merged pull request #41:
URL: https://github.com/apache/arrow-cookbook/pull/41
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-cookbook] nealrichardson commented on a change in pull request #41: [R] Remove unnecessary head()s
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #41:
URL: https://github.com/apache/arrow-cookbook/pull/41#discussion_r692182956
##########
File path: r/content/reading_and_writing_data.Rmd
##########
@@ -60,11 +60,11 @@ Given a Parquet file, it can be read back in by using `arrow::read_parquet()`.
```{r, read_parquet}
parquet_tbl <- read_parquet("my_table.parquet")
-head(parquet_tbl)
+parquet_tbl
```
```{r, test_read_parquet, opts.label = "test"}
test_that("read_parquet works as expected", {
- expect_equivalent(dplyr::collect(parquet_tbl), tibble::tibble(group = c("A", "B", "C"), score = c(99, 97, 99)))
+ expect_identical(as.data.frame(parquet_tbl), tibble::tibble(group = c("A", "B", "C"), score = c(99, 97, 99)))
Review comment:
Ah, I know what it is: `parquet_tbl` is already a `tbl_df` so `dplyr::collect()` literally does nothing, and `as.data.frame.tbl_df` removes the attributes. So it's not that we should do as.data.frame() instead of collect(), we should do neither (and consider renaming the variable if `_tbl` is ambiguous).
```suggestion
expect_identical(parquet_tbl, tibble::tibble(group = c("A", "B", "C"), score = c(99, 97, 99)))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org