You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "thisisnic (via GitHub)" <gi...@apache.org> on 2023/06/06 16:50:16 UTC
[GitHub] [arrow] thisisnic opened a new pull request, #35955: GH-35949: [R] File reader options class objects should print the selected values
thisisnic opened a new pull request, #35955:
URL: https://github.com/apache/arrow/pull/35955
(no comment)
--
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] paleolimbot commented on a diff in pull request #35955: GH-35949: [R] File reader options class objects should print the selected values
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #35955:
URL: https://github.com/apache/arrow/pull/35955#discussion_r1220048738
##########
r/R/csv.R:
##########
@@ -464,10 +464,22 @@ CsvTableReader$create <- function(file,
CsvReadOptions <- R6Class("CsvReadOptions",
inherit = ArrowObject,
public = list(
- encoding = NULL
+ encoding = NULL,
+ print = function(...) {
+ cat("CsvReadOptions")
+ cat("\nencoding:", self$encoding)
+ cat("\ncolumn_names:", self$column_names)
+ cat("\nblock_size:", self$block_size)
+ cat("\nskip_rows:", self$skip_rows)
+ cat("\nautogenerate_column_names:", self$autogenerate_column_names)
Review Comment:
Probably a `for()` loop here?
```r
for (attr in c("encoding", ...)) {
cat(sprintf("%s: %s\n", attr, self[[attr]]))
}
```
--
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] paleolimbot commented on a diff in pull request #35955: GH-35949: [R] CSV File reader options class objects should print the selected values
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #35955:
URL: https://github.com/apache/arrow/pull/35955#discussion_r1224240655
##########
r/tests/testthat/test-dataset-csv.R:
##########
@@ -574,3 +574,11 @@ test_that("open_delim_dataset params passed through to open_dataset", {
expect_equal(ds$time, "16-01-2023")
})
+
Review Comment:
One more test here for the data access part (as opposed to the printing part)? Like:
```r
test_that("CSV options field access", {
options <- CsvReadOptions$create(skip_rows = 102)
expect_identical(options$skip_rows, 102)
# ... for all the options we just added the ability to access
})
```
...for each option we added a
--
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] thisisnic merged pull request #35955: GH-35949: [R] CSV File reader options class objects should print the selected values
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic merged PR #35955:
URL: https://github.com/apache/arrow/pull/35955
--
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] thisisnic commented on a diff in pull request #35955: GH-35949: [R] File reader options class objects should print the selected values
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #35955:
URL: https://github.com/apache/arrow/pull/35955#discussion_r1220065487
##########
r/R/csv.R:
##########
@@ -464,10 +464,22 @@ CsvTableReader$create <- function(file,
CsvReadOptions <- R6Class("CsvReadOptions",
inherit = ArrowObject,
public = list(
- encoding = NULL
+ encoding = NULL,
+ print = function(...) {
+ cat("CsvReadOptions")
+ cat("\nencoding:", self$encoding)
+ cat("\ncolumn_names:", self$column_names)
+ cat("\nblock_size:", self$block_size)
+ cat("\nskip_rows:", self$skip_rows)
+ cat("\nautogenerate_column_names:", self$autogenerate_column_names)
Review Comment:
Thanks! I was also checking in that there's no way of doing the other bits more programmatically, but doesn't appear to be, so I'll just plough on!
--
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] ursabot commented on pull request #35955: GH-35949: [R] CSV File reader options class objects should print the selected values
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #35955:
URL: https://github.com/apache/arrow/pull/35955#issuecomment-1586270999
Benchmark runs are scheduled for baseline = 788760570d8182e1eb05a47e8871360515f431be and contender = fe6093228fd27e902b83031d09eef2765d615ed7. fe6093228fd27e902b83031d09eef2765d615ed7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0% :warning: Contender and baseline run contexts do not match] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/0cee706745254baf8675f0ef51c92e34...8b17203e77cb4ea6a583e98649e2d9e8/)
[Finished :arrow_down:0.74% :arrow_up:0.03%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/1626c471d20745cf8532d232b883603d...2846772bc9dd471480d7aa1f794ef8ff/)
[Finished :arrow_down:0.65% :arrow_up:0.0% :warning: Contender and baseline run contexts do not match] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/c42d83ded29d481496262262a3159081...9a3dc82f9ae9438fa76f79dc52d810b3/)
[Finished :arrow_down:3.01% :arrow_up:0.3% :warning: Contender and baseline run contexts do not match] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/d35b6b1646354ae8901286fa0cd8645b...ab5349fb62f942e2b2b25552f096ab02/)
Buildkite builds:
[Finished] [`fe609322` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/3020)
[Finished] [`fe609322` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/3056)
[Finished] [`fe609322` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/3021)
[Finished] [`fe609322` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/3046)
[Finished] [`78876057` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/3019)
[Finished] [`78876057` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/3055)
[Finished] [`78876057` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/3020)
[Finished] [`78876057` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/3045)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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] thisisnic commented on pull request #35955: GH-35949: [R] File reader options class objects should print the selected values
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on PR #35955:
URL: https://github.com/apache/arrow/pull/35955#issuecomment-1579124729
I started on this as it'd be useful for some debugging I'm doing, though I wonder if there's a more efficient way of doing this than copying the code for each attribute. Perhaps not?
--
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] thisisnic commented on a diff in pull request #35955: GH-35949: [R] CSV File reader options class objects should print the selected values
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #35955:
URL: https://github.com/apache/arrow/pull/35955#discussion_r1225581214
##########
r/tests/testthat/test-dataset-csv.R:
##########
@@ -574,3 +574,11 @@ test_that("open_delim_dataset params passed through to open_dataset", {
expect_equal(ds$time, "16-01-2023")
})
+
Review Comment:
Added
--
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