-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(r): Improve vctr class integration #79
Changes from 5 commits
07ea626
16eb388
03c37da
2566162
7378b50
2bc7f2f
7320d91
05c2a39
3e55dd9
2611e15
acd0113
52d51d2
3bb11f0
724146d
57ad53c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
|
||
register_geoarrow_extension <- function() { | ||
for (ext_name in geoarrow_extension_name_all()) { | ||
nanoarrow::register_nanoarrow_extension( | ||
ext_name, | ||
nanoarrow::nanoarrow_extension_spec(subclass = "geoarrow_extension_spec") | ||
) | ||
} | ||
} | ||
|
||
#' @importFrom nanoarrow infer_nanoarrow_ptype_extension | ||
#' @export | ||
infer_nanoarrow_ptype_extension.geoarrow_extension_spec <- function(extension_spec, x, ...) { | ||
new_geoarrow_vctr(list(), x, integer()) | ||
} | ||
|
||
#' @importFrom nanoarrow convert_array_extension | ||
#' @export | ||
convert_array_extension.geoarrow_extension_spec <- function(extension_spec, | ||
array, to, ...) { | ||
# For the default, this will dispatch to convert_array.geoarrow_vctr(). | ||
# This gets called if to is a base R type (e.g., integer()) | ||
stop( | ||
sprintf( | ||
"Can't convert geoarrow extension array to object of class '%s'", | ||
class(to)[1] | ||
) | ||
) | ||
} | ||
|
||
#' @importFrom nanoarrow as_nanoarrow_array_extension | ||
#' @export | ||
as_nanoarrow_array_extension.geoarrow_extension_spec <- function( | ||
extension_spec, x, ..., schema = NULL) { | ||
as_geoarrow_array(x, schema = schema) | ||
} | ||
|
||
#' @importFrom nanoarrow convert_array | ||
#' @export | ||
convert_array.geoarrow_vctr <- function(array, to, ...) { | ||
as_geoarrow_vctr(array, schema = as_nanoarrow_schema(to)) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,6 +112,60 @@ infer_nanoarrow_schema.wk_xy <- function(x, ...) { | |
wk_geoarrow_schema(x, na_extension_geoarrow, "POINT", dimensions = dims) | ||
} | ||
|
||
#' @export | ||
convert_array.wk_wkt <- function(array, to, ...) { | ||
schema <- infer_nanoarrow_schema(array) | ||
geo <- geoarrow_schema_parse(schema) | ||
vctr <- as_geoarrow_vctr(array) | ||
|
||
if (geo$extension_name == "geoarrow.wkt") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For a future pr, I think we could extract some of this into There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then these could be skeletons. Or the reverse, and as_ methods are just calling these methods. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They almost certainly should be optimized! I'm pretty sure they just go through the handler right now (but at least they work!): library(wk)
library(geoarrow)
as_xy(as_geoarrow_vctr("POINT (0 1)"))
#> <wk_xy[1]>
#> [1] (0 1)
as_wkt(as_geoarrow_vctr("POINT (0 1)"))
#> <wk_wkt[1]>
#> [1] POINT (0 1)
as_wkb(as_geoarrow_vctr("POINT (0 1)"))
#> <wk_wkb[1]>
#> [1] <POINT (0 1)> Created on 2023-12-01 with reprex v2.0.2 |
||
out <- wk::new_wk_wkt(convert_array(force_array_storage(array))) | ||
} else { | ||
out <- wk::wk_handle(vctr, wk::wkt_writer()) | ||
} | ||
|
||
wk::wk_crs(out) <- wk::wk_crs_output(vctr, to) | ||
wk::wk_is_geodesic_output(vctr, to) | ||
out | ||
paleolimbot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
#' @export | ||
convert_array.wk_wkb <- function(array, to, ...) { | ||
schema <- infer_nanoarrow_schema(array) | ||
geo <- geoarrow_schema_parse(schema) | ||
vctr <- as_geoarrow_vctr(array) | ||
|
||
if (geo$extension_name == "geoarrow.wkb") { | ||
storage <- convert_array(force_array_storage(array)) | ||
# Comes back as a blob::blob | ||
attributes(storage) <- NULL | ||
out <- wk::new_wk_wkb(storage) | ||
} else { | ||
out <- wk::wk_handle(vctr, wk::wkb_writer()) | ||
} | ||
|
||
wk::wk_crs(out) <- wk::wk_crs_output(vctr, to) | ||
wk::wk_is_geodesic_output(vctr, to) | ||
paleolimbot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
out | ||
} | ||
|
||
#' @export | ||
convert_array.wk_xy <- function(array, to, ...) { | ||
schema <- infer_nanoarrow_schema(array) | ||
geo <- geoarrow_schema_parse(schema) | ||
vctr <- as_geoarrow_vctr(array) | ||
|
||
if (geo$extension_name == "geoarrow.point") { | ||
out <- wk::as_xy(convert_array(force_array_storage(array))) | ||
} else { | ||
out <- wk::wk_handle(vctr, wk::xy_writer()) | ||
} | ||
|
||
wk::wk_crs(out) <- wk::wk_crs_output(vctr, to) | ||
wk::wk_is_geodesic(out) <- wk::wk_is_geodesic_output(vctr, to) | ||
wk::as_xy(out, dims = names(unclass(to))) | ||
} | ||
|
||
wk_geoarrow_schema <- function(x, type_constructor, ...) { | ||
if (inherits(x, c("nanoarrow_array", "nanoarrow_array_stream"))) { | ||
schema <- nanoarrow::infer_nanoarrow_schema(x) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
|
||
# nocov start | ||
.onLoad <- function(...) { | ||
register_geoarrow_extension() | ||
|
||
s3_register("sf::st_as_sfc", "geoarrow_vctr") | ||
} | ||
|
||
# From the `vctrs` package (this function is intended to be copied | ||
# without attribution or license requirements to avoid a hard dependency on | ||
# vctrs: | ||
# https://github.com/r-lib/vctrs/blob/c2a7710fe55e3a2249c4fdfe75bbccbafcf38804/R/register-s3.R#L25-L31 | ||
s3_register <- function(generic, class, method = NULL) { | ||
stopifnot(is.character(generic), length(generic) == 1) | ||
stopifnot(is.character(class), length(class) == 1) | ||
|
||
pieces <- strsplit(generic, "::")[[1]] | ||
stopifnot(length(pieces) == 2) | ||
package <- pieces[[1]] | ||
generic <- pieces[[2]] | ||
|
||
caller <- parent.frame() | ||
|
||
get_method_env <- function() { | ||
top <- topenv(caller) | ||
if (isNamespace(top)) { | ||
asNamespace(environmentName(top)) | ||
} else { | ||
caller | ||
} | ||
} | ||
get_method <- function(method, env) { | ||
if (is.null(method)) { | ||
get(paste0(generic, ".", class), envir = get_method_env()) | ||
} else { | ||
method | ||
} | ||
} | ||
|
||
register <- function(...) { | ||
envir <- asNamespace(package) | ||
|
||
# Refresh the method each time, it might have been updated by | ||
# `devtools::load_all()` | ||
method_fn <- get_method(method) | ||
stopifnot(is.function(method_fn)) | ||
|
||
|
||
# Only register if generic can be accessed | ||
if (exists(generic, envir)) { | ||
registerS3method(generic, class, method_fn, envir = envir) | ||
} else if (identical(Sys.getenv("NOT_CRAN"), "true")) { | ||
warning(sprintf( | ||
"Can't find generic `%s` in package %s to register S3 method.", | ||
generic, | ||
package | ||
)) | ||
} | ||
} | ||
|
||
# Always register hook in case package is later unloaded & reloaded | ||
setHook(packageEvent(package, "onLoad"), register) | ||
|
||
# Avoid registration failures during loading (pkgload or regular) | ||
if (isNamespaceLoaded(package)) { | ||
register() | ||
} | ||
|
||
invisible() | ||
} | ||
# nocov end |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we prefer
wk::wk_set_crs(sfc, wk::wk_crs(x))
here?In theory, this change reduces geoarrow's knowledge of sf by deferring that responsibility to wk. In reality, it's the same code and still requires sf to be installed, so maybe it doesn't matter.