Handling NA input values about googlepolylines HOT 6 CLOSED

Thanks for the examples!

decode

Should this return NA_real_, 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 :)

encode

Yes these cases need to be handled. My first thoughts are to either

1. Issue an Error if any NAs are found, to force the issue back to the user
2. Issue a warning if any NAs are found, but exclude them from the encoding

What do you think to these options?

from googlepolylines.

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?

from googlepolylines.

but are you interested in a performance/refactor PR as well?

absolutely! Always keen to see & learn from others.

from googlepolylines.

@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

from googlepolylines.

Nope, I addressed the decode() example, and made some minor speed refactors, but did not address the encode() examples.

from googlepolylines.

Moved the encode() examples to a separate issue - #39

from googlepolylines.