Closed ChrisMuir closed 5 years ago
Thanks for the examples!
Should this return NAreal, or a data frame containing two NA values
My instinct tells me a data.frame with two NA values, to keep the structure consistent. Which do you think is the better option?
I'd be happy to submit a PR for this if you'd like.
yes please :)
Yes these cases need to be handled. My first thoughts are to either
Error
if any NA
s are found, to force the issue back to the userwarning
if any NA
s are found, but exclude them from the encodingWhat do you think to these options?
So full disclaimer, I actually have never used googlePolylines, so I don't have a strong take what makes the most sense on how to handle these cases. I was just poking around this repo after seeing the cool stuff being done with mapdeck
on rstats Twitter, and seeing that you were planning on trying to migrate some of the code to Rcpp.
Having said that, for the decode example, I agree that returning a data frame is probably better. Looking at the list output, I could see wanting to rbind
all of the results together via do.call
, so a data frame with the same column headers seems wise. I'll submit a PR for that fix tomorrow.
Also, I was playing around with the decode Rcpp functions, and modifyed it so that it's not having to create a new Rcpp::DataFrame
for every input string......I currently have a ~15x speed up for decode()
with char vector input, and all the tests are passing. Still want to work with it some more, but are you interested in a performance/refactor PR as well?
but are you interested in a performance/refactor PR as well?
absolutely! Always keen to see & learn from others.
@ChrisMuir did you look at the encode
examples while you were writing your PR?
If not I might move them into a separate issue and make them part of the v0.7.3 milestone
Nope, I addressed the decode()
example, and made some minor speed refactors, but did not address the encode()
examples.
Moved the encode()
examples to a separate issue - https://github.com/SymbolixAU/googlePolylines/issues/39
I ran into a few instances of
encode()
anddecode()
handling NA inputs in interesting ways. Here are some examples:decode()
Should this return
NA_real_
, or a data frame containing two NA values? Either could be done within the Rcpp functionrcpp_decode_polyline()
without much trouble or additional code. I'd be happy to submit a PR for this if you'd like.encode()
Example 1:
NA
inputs round-tripped throughencode/decode
return numeric lat/lon coordinates.Example 2: Data frame with four observations round-tripped through
encode/decode
returns a data frame with only three observations.