ARROW-14258: [R] Warn if an SF column is made into a table#11361
ARROW-14258: [R] Warn if an SF column is made into a table#11361jonkeane wants to merge 4 commits into
Conversation
|
|
|
|
||
| attempt_to_save_row_level <- getOption("arrow.preserve_row_level_metadata", FALSE) && | ||
| is.list(x) && !inherits(x, "POSIXlt") | ||
| if (attempt_to_save_row_level) { |
There was a problem hiding this comment.
You could move the warning to an else if (inherits(x, c("sfc", "sf"))) below this, maybe that removes the complexity warning? Though maybe we should warn regardless of whether we're saving the metadata, just a different warning?
| "One of the columns given appears to be an `sfc` SF column. Due to their unique ", | ||
| "nature, these columns do not convert to Arrow well. We are working on ", | ||
| "better ways to do this, but in the interim we recommend converting any `sfc` ", | ||
| "columns to WKB (well-known binary) columns before using them with Arrow.", |
There was a problem hiding this comment.
Can you suggest a function in sf that does this?
There was a problem hiding this comment.
We could also talk about sfarrow here (or maybe just in the news, since it'll be easier to link to?)
| ) | ||
| withr::with_options( | ||
| list("arrow.preserve_row_level_metadata" = TRUE), { | ||
| withr::local_options(list("arrow.preserve_row_level_metadata" = TRUE)) |
There was a problem hiding this comment.
It looks like you're setting the option twice?
There was a problem hiding this comment.
Ah yes, that was a mistake, fixed now.
| object_length_linter = object_length_linter(40), | ||
| object_usage_linter = NULL, # R6 methods are flagged, | ||
| cyclocomp_linter = cyclocomp_linter(26) # TODO: reduce to default of 15 | ||
| cyclocomp_linter = cyclocomp_linter(20) # TODO: reduce to default of 15 |
There was a problem hiding this comment.
This appears to trigger a new annotation
There was a problem hiding this comment.
Oops, I got too eager that the metadata function was our only excessively cyclocomplex function. I'll bump this back up
nealrichardson
left a comment
There was a problem hiding this comment.
Two notes but otherwise LGTM
|
Benchmark runs are scheduled for baseline = c875c5d and contender = 1fa1a2b. 1fa1a2b is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
No description provided.