Skip to content
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

Implement promote_to_multi when converting WKB to sfc #2369

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

paleolimbot
Copy link
Contributor

@paleolimbot paleolimbot commented Mar 31, 2024

Apologies for taking so long to circle back here, but this is one approach to ensuring that R CMD check passes when R_SF_ST_READ_USE_STREAM=true. It also has the nice side-effect that use_stream = TRUE no longer uses the wk package to construct sfc objects and that all users of st_as_sfc(<WKB>) can now use promote_to_multi = TRUE to get the same read-ogr-like behaviour.

Closes #2296.

Example:

library(sf)

vec <- st_sfc(
    st_point(c(0, 1)),
    st_point(),
    st_point(c(2, 3)),
    st_multipoint(matrix(c(4, 5), ncol = 2))
)

wkb_vec <- st_as_binary(vec)

st_as_sfc(wkb_vec, promote_to_multi = FALSE)
st_as_sfc(wkb_vec, promote_to_multi = TRUE)

I do still see:

Failure (test-tm.R:20:3): st_read and write handle date and time
x[["tm"]] not equal to x2[["tm"]].
Attributes: < Component “tzone”: 1 string mismatch >

Failure (test-tm.R:40:3): st_read and write handle date and time
x[["tm"]] not equal to x2[["tm"]].
Attributes: < Component “tzone”: 1 string mismatch >

...when running testthat::test_local() with R_SF_ST_READ_USE_STREAM=true. I think this happens because of how nanoarrow converts timestamps without an explicit timezone to R objects: nanoarrow assigns UTC as opposed to setting tzone = "" or omitting it. I copied this behaviour from readr because it is more reproducible between systems, but I'm not sure it's any more or less correct.

@paleolimbot paleolimbot changed the title Implement promote_multi when converting WKB to sfc Implement promote_to_multi when converting WKB to sfc Mar 31, 2024
@paleolimbot paleolimbot marked this pull request as ready for review March 31, 2024 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

R CMD check with R_SF_ST_READ_USE_STREAM=true
1 participant