Refactor encode() Rcpp code about googlepolylines HOT 11 CLOSED

no you can leave / ignore that - it's not correct. I was going to re-do the Z/M stuff.

from googlepolylines.

Awesome stuff.

Can you please open a new PR (for no particular reason, just seems logical to keep the decode() and encode() PRs separate)

from googlepolylines.

Cool, sounds good and will do. Still tweaking a few things, but should have things done in a day or two.

from googlepolylines.

Okay, so this may end up taking longer than I thought....I ran into git issues tonight, and my wife and I are moving this weekend, so I probably won't be able to do anything with this until after the move and we're settled in, so will probably be late next week.

To summarize the git issues, I created a new branch for the new pull request, rcpp_encode_refactor, added your repo as a remote, and tried to rebase to your repo's upstream master. However, when pushing my newest edits to my new branch on GitHub, I see that the new branch still has all four commits I made earlier this week. and if I try to open a new PR, it also contains all four commits from my original PR.

Not sure what to do with this other than creating another new branch and trying again. Also, if you were to merge my original pull request, I don't know what effect that would have on my new branch.

Sorry for all the troubles....sometimes git is hard 😄

from googlepolylines.

ha - been there!

Would it help if I first merged your original PR over the weekend?

from googlepolylines.

Yeah, I mean, in theory if you were to merge the original PR, then a new PR from my new branch should only contain my latest commit.

And if you want edits/changes to the original PR, please don't hesitate to ask, the master branch of my fork is free of the encode() refactor edits/commit, so I'd be able to push any follow up edits from master to the original PR.

Thanks!

from googlepolylines.

Also, one last question....in the file encode.cpp, there's a bunch of code related to objects that have Z & M attributes that is commented out (see encode_vector() example below). Do you want me to update that code to be compatible with the new code that I added (while still leaving it commented out)?

void encode_vector( std::ostringstream& os, std::ostringstream& oszm, Rcpp::List vec, Rcpp::CharacterVector& sfg_dim,
                    int dim_divisor) {

  // - XY == [0][1]
  // - XYZ == [0][1][2]{3}
  // - XYM == [0][1][2]{3}
  // - XYZM == [0][1][2][3]
  
  int n = vec.size() / dim_divisor;
  
  global_vars::lats.clear();
  global_vars::lons.clear();
  
  for (int i = 0; i < n; i++) {
    global_vars::lons.push_back(vec[i]);
    global_vars::lats.push_back(vec[(i + n)]);
  }
  
  global_vars::encodedString = encode_polyline();
  addToStream(os);
  
  // if (dim_divisor > 2) {
  //   // there are Z and M attributes to encode
  //   // constructor sets them to 0
  //   Rcpp::NumericVector elev(n);
  //   Rcpp::NumericVector meas(n);
  //   
  //   for (int i = 0; i < n; i++) {
  //     elev[i] = vec[(i + n + n)];
  //     meas[i] = dim_divisor == 4 ? vec[(i + n + n + n)] : 0;
  //   }
  //   encodedString = encode_polyline(elev, meas);
  //   addToStream(oszm, encodedString);
  // }
  
}

from googlepolylines.

Merged both PRs, and added you as a contributor.

Are you happy to close this issue, or is there more to come?

from googlepolylines.

Cool, and thanks for adding me as a contributor!

Nope that's all I have for now. I enjoyed working on this quite a bit, thanks for letting me contribute 👍

from googlepolylines.

no no, thank you!

and a bit of advertising for your work :) https://twitter.com/symbolixAU/status/1045537693915000832

from googlepolylines.

Woah that's super cool! Thanks for the shout out!

from googlepolylines.