You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "jonkeane (via GitHub)" <gi...@apache.org> on 2023/10/20 18:47:11 UTC
[PR] GH-38376 [R]: Add `dimnames` method to `Dataset` class [arrow]
jonkeane opened a new pull request, #38377:
URL: https://github.com/apache/arrow/pull/38377
### Rationale for this change
Add `dimnames` method so that things like `colnames(ds)` work out of the box on datasets. Inspired by dbplyr's lazy tbl implementation
### What changes are included in this PR?
An additional `dimnames` method for Dataset objects
### Are these changes tested?
Yes
### Are there any user-facing changes?
Yes
--
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
Re: [PR] GH-38376 [R]: Add `dimnames` method to `Dataset` class [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #38377:
URL: https://github.com/apache/arrow/pull/38377#issuecomment-1775063030
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit f489996e707dd136e41f1eb8ed963f707eed18ad.
There was 1 benchmark result indicating a performance regression:
- Commit Run on `ursa-i9-9960x` at [2023-10-22 17:59:47Z](https://conbench.ursa.dev/compare/runs/b97ffbffbe28497d9cda75b124bfb704...779a94ec29b649c29c5e4d1be968d6e0/)
- [`tpch` (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-01, scale_factor=1](https://conbench.ursa.dev/compare/benchmarks/0653257b9d937c03800045a8878f304c...06535721494e7bc2800031651ae9f935)
The [full Conbench report](https://github.com/apache/arrow/runs/17960200494) has more details.
--
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
Re: [PR] GH-38376 [R]: Add `dimnames` method to `Dataset` class [arrow]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #38377:
URL: https://github.com/apache/arrow/pull/38377#discussion_r1367802343
##########
r/R/dataset.R:
##########
@@ -527,6 +527,9 @@ names.Dataset <- function(x) names(x$schema)
#' @export
dim.Dataset <- function(x) c(x$num_rows, x$num_cols)
+#' @export
+dimnames.Dataset <- function(x) list(NULL, names(x))
Review Comment:
I think copying dbplyr's behaviour is the best route. Is this already handled for `Table`?
--
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
Re: [PR] GH-38376 [R]: Add `dimnames` method to `Dataset` class [arrow]
Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on code in PR #38377:
URL: https://github.com/apache/arrow/pull/38377#discussion_r1367924767
##########
r/R/dataset.R:
##########
@@ -527,6 +527,9 @@ names.Dataset <- function(x) names(x$schema)
#' @export
dim.Dataset <- function(x) c(x$num_rows, x$num_cols)
+#' @export
+dimnames.Dataset <- function(x) list(NULL, names(x))
Review Comment:
Yeah, it's not immediately obvious, but Tables all inherit `ArrowTabular` which has `dimnames.ArrowTabular` defined already
--
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
Re: [PR] GH-38376 [R]: Add `dimnames` method to `Dataset` class [arrow]
Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on code in PR #38377:
URL: https://github.com/apache/arrow/pull/38377#discussion_r1367465051
##########
r/R/dataset.R:
##########
@@ -527,6 +527,9 @@ names.Dataset <- function(x) names(x$schema)
#' @export
dim.Dataset <- function(x) c(x$num_rows, x$num_cols)
+#' @export
+dimnames.Dataset <- function(x) list(NULL, names(x))
Review Comment:
We might consider a message here that tells folks that the `NULL` doesn't mean there are none. But then again dbplyr doesn't do that!
--
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
Re: [PR] GH-38376 [R]: Add `dimnames` method to `Dataset` class [arrow]
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic merged PR #38377:
URL: https://github.com/apache/arrow/pull/38377
--
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