Added gh_fill.R#35
Conversation
Initial add of function to fill geohash with contained geohashes
Added gh_fill via upload
| @@ -0,0 +1,28 @@ | |||
| #' Fill geohash prefix with members | |||
There was a problem hiding this comment.
The package isn't currently using roxygen2, so you'll need to either write the .Rd directly, or convert the rest of the package to use roxygen2.
|
|
||
|
|
||
| gh_fill <- function(geohashes, precision) { | ||
| if (uniqueN(nchar(geohashes)) > 1) { |
There was a problem hiding this comment.
uniqueN is a data.table function, which we don't import. length(unique()) should be fine here.
| unlist(strsplit("0123456789bcdefghjkmnpqrstuvwxyz", split = "")) | ||
|
|
||
| grid <- | ||
| do.call(data.table::CJ, append(list(geohashes), replicate(new_levels, base32, FALSE))) |
There was a problem hiding this comment.
could you do a performance comparison of CJ() vs. expand.grid() for typical inputs? E.g. 1e4 inputs and adding 1-3 characters of precision...
Depending on that we'll add the data.table dependency
| if (uniqueN(nchar(geohashes)) > 1) { | ||
| stop("Input Geohashes must all have the same precision level.") | ||
| } | ||
| if (sum(grepl("['ailo]", geohashes)) > 0) { |
There was a problem hiding this comment.
prefer any(grepl()).
also, elsewhere in the package, we allow upper-case input, we should strive to match case here.
| #' @param geohashes Character vector of input geohashes. They must all be of same precision | ||
| #' @param precision Positive integer scalar controlling the 'zoom level' – how many characters should be used in the output. | ||
| #' @return Character vector of geohashes corresponding to the input. | ||
| #' @export |
There was a problem hiding this comment.
please add examples and tests, it will especially make it easier to review when I can see expected inputs/outputs
|
|
||
| new_levels <- precision - nchar(geohashes[1]) | ||
|
|
||
| base32 <- |
There was a problem hiding this comment.
it's probably better to run this at compile-time in the package environment, rather than every time gh_fill() is invoked
|
Thanks Garnet! Sorry for the long delay. I no longer work with geohashes very much so this package has lost priority (open to new maintainers!). If I understand right, the function would indeed be useful. Needs some package "productionization" stuff, and I'd like to justify adding |
|
Friendly ping if you'd like this merged, I've got a new version ready to submit soon. |
|
Hi Michael, I have added the changes that you referenced in the earlier messages. I hope it's been done in an okay manner. I wrote a test to compare data.table::CJ() vs base::expand.grid. Often when I use this function it is to create vectors in the 10's of millions. I'm just not sure if the speed increase warrants a dependency on data.table. |
2 participants
Added a function to fill geohashes with the contained geohashes.