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/03/02 18:58:06 UTC
[GitHub] [arrow] pachamaltese opened a new pull request #9618: ARROW-11582: [R] write_dataset 'format' argument default and validati…
pachamaltese opened a new pull request #9618:
URL: https://github.com/apache/arrow/pull/9618
…on could be better
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pachamaltese commented on pull request #9618: ARROW-11582: [R] write_dataset 'format' argument default and validation could be better
Posted by GitBox <gi...@apache.org>.
pachamaltese commented on pull request #9618:
URL: https://github.com/apache/arrow/pull/9618#issuecomment-789297605
I think it can be merged now, since the default is "parquet"
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson closed pull request #9618: ARROW-11582: [R] write_dataset 'format' argument default and validation could be better
Posted by GitBox <gi...@apache.org>.
nealrichardson closed pull request #9618:
URL: https://github.com/apache/arrow/pull/9618
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pachamaltese commented on a change in pull request #9618: ARROW-11582: [R] write_dataset 'format' argument default and validati…
Posted by GitBox <gi...@apache.org>.
pachamaltese commented on a change in pull request #9618:
URL: https://github.com/apache/arrow/pull/9618#discussion_r585876862
##########
File path: r/tests/testthat/test-dataset.R
##########
@@ -1061,6 +1061,17 @@ test_that("Writing a dataset: Parquet->Parquet (default)", {
)
})
+test_that("Writing a dataset: Not specified format", {
+ skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-9651
+ dst_dir <- make_temp_dir()
+ write_dataset(mtcars, dst_dir)
+ new_ds <- open_dataset(dst_dir)
Review comment:
sure, changing now
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9618: ARROW-11582: [R] write_dataset 'format' argument default and validation could be better
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9618:
URL: https://github.com/apache/arrow/pull/9618#issuecomment-789254059
https://issues.apache.org/jira/browse/ARROW-11582
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pachamaltese commented on a change in pull request #9618: ARROW-11582: [R] write_dataset 'format' argument default and validation could be better
Posted by GitBox <gi...@apache.org>.
pachamaltese commented on a change in pull request #9618:
URL: https://github.com/apache/arrow/pull/9618#discussion_r585911523
##########
File path: r/R/dataset-write.R
##########
@@ -56,11 +56,12 @@
#' @export
write_dataset <- function(dataset,
path,
- format = dataset$format,
+ format = c("parquet", "feather", "arrow", "ipc"),
Review comment:
how about this?
![Uploading image.png…]()
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a change in pull request #9618: ARROW-11582: [R] write_dataset 'format' argument default and validati…
Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #9618:
URL: https://github.com/apache/arrow/pull/9618#discussion_r585844378
##########
File path: r/tests/testthat/test-dataset.R
##########
@@ -1061,6 +1061,17 @@ test_that("Writing a dataset: Parquet->Parquet (default)", {
)
})
+test_that("Writing a dataset: Not specified format", {
+ skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-9651
+ dst_dir <- make_temp_dir()
+ write_dataset(mtcars, dst_dir)
+ new_ds <- open_dataset(dst_dir)
Review comment:
Should we assert that this has produced parquet files / the dataset as opened is parquet files?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on a change in pull request #9618: ARROW-11582: [R] write_dataset 'format' argument default and validati…
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #9618:
URL: https://github.com/apache/arrow/pull/9618#discussion_r585902139
##########
File path: r/R/dataset-write.R
##########
@@ -56,11 +56,12 @@
#' @export
write_dataset <- function(dataset,
path,
- format = dataset$format,
+ format = c("parquet", "feather", "arrow", "ipc"),
Review comment:
Need to update the docs to reflect the new behavior. You may (also) want to borrow the language used to describe `format` from `dataset-format.R`.
##########
File path: r/tests/testthat/test-dataset.R
##########
@@ -1061,6 +1061,17 @@ test_that("Writing a dataset: Parquet->Parquet (default)", {
)
})
+test_that("Writing a dataset: Not specified format", {
+ skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-9651
+ dst_dir <- make_temp_dir()
+ write_dataset(mtcars, dst_dir)
+ new_ds <- open_dataset(dst_dir)
Review comment:
FWIW the default format for open_dataset() is parquet, so this test would have failed if it hadn't written parquet files.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a change in pull request #9618: ARROW-11582: [R] write_dataset 'format' argument default and validati…
Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #9618:
URL: https://github.com/apache/arrow/pull/9618#discussion_r585844545
##########
File path: r/tests/testthat/test-dataset.R
##########
@@ -1061,6 +1061,17 @@ test_that("Writing a dataset: Parquet->Parquet (default)", {
)
})
+test_that("Writing a dataset: Not specified format", {
Review comment:
```suggestion
test_that("Writing a dataset: no format specified", {
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pachamaltese commented on a change in pull request #9618: ARROW-11582: [R] write_dataset 'format' argument default and validation could be better
Posted by GitBox <gi...@apache.org>.
pachamaltese commented on a change in pull request #9618:
URL: https://github.com/apache/arrow/pull/9618#discussion_r585911523
##########
File path: r/R/dataset-write.R
##########
@@ -56,11 +56,12 @@
#' @export
write_dataset <- function(dataset,
path,
- format = dataset$format,
+ format = c("parquet", "feather", "arrow", "ipc"),
Review comment:
how about 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pachamaltese commented on a change in pull request #9618: ARROW-11582: [R] write_dataset 'format' argument default and validati…
Posted by GitBox <gi...@apache.org>.
pachamaltese commented on a change in pull request #9618:
URL: https://github.com/apache/arrow/pull/9618#discussion_r585886725
##########
File path: r/tests/testthat/test-dataset.R
##########
@@ -1061,6 +1061,17 @@ test_that("Writing a dataset: Parquet->Parquet (default)", {
)
})
+test_that("Writing a dataset: Not specified format", {
+ skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-9651
+ dst_dir <- make_temp_dir()
+ write_dataset(mtcars, dst_dir)
+ new_ds <- open_dataset(dst_dir)
Review comment:
updated in adf82e4
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on a change in pull request #9618: ARROW-11582: [R] write_dataset 'format' argument default and validation could be better
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #9618:
URL: https://github.com/apache/arrow/pull/9618#discussion_r585918333
##########
File path: r/R/dataset-write.R
##########
@@ -56,11 +56,12 @@
#' @export
write_dataset <- function(dataset,
path,
- format = dataset$format,
+ format = c("parquet", "feather", "arrow", "ipc"),
Review comment:
xref to FileFormat is good (if it works, does it need the `()`? FileFormat is not callable) though I think it's important to (still) note that only parquet and feather are supported.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pachamaltese commented on a change in pull request #9618: ARROW-11582: [R] write_dataset 'format' argument default and validation could be better
Posted by GitBox <gi...@apache.org>.
pachamaltese commented on a change in pull request #9618:
URL: https://github.com/apache/arrow/pull/9618#discussion_r585949781
##########
File path: r/R/dataset-write.R
##########
@@ -56,11 +56,12 @@
#' @export
write_dataset <- function(dataset,
path,
- format = dataset$format,
+ format = c("parquet", "feather", "arrow", "ipc"),
Review comment:
fixed! the link it works without ()
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org