cell_index: Add IntersectingLabels and VisitIntersectingCells #98
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
@jmr what else is necessary to make this mergeable? Do you want me to review? |
| rangeIter.Begin() | ||
|
|
||
| for ok := true; ok; ok = pos != len(target) { | ||
| if rangeIter.LimitID() <= target[pos].RangeMin() { |
There was a problem hiding this comment.
In C++ this was if rangeIter.LimitID() <= rangeIter.RangeMin() { is there a reason it's not like that here?
There was a problem hiding this comment.
In C++ it was range.limit_id() <= it->range_min() where it is an iterator derived from target. Because there's no iterator I manually iterate over the cell ids.
There was a problem hiding this comment.
| id = id.ChildBegin() | ||
| } | ||
| } | ||
| target.Normalize() |
There was a problem hiding this comment.
The call to Normalize wasn't in the C++. Does removing this change the test outcomes?
There was a problem hiding this comment.
Removing target.Normalize() fails the test. I can't remember why anymore (my bad, left this open for too long).
There was a problem hiding this comment.
I think this is because S2CellUnion is a class in C++ and its constructor calls Normalize().
rubenpoppe
force-pushed
the
feature/cell-index
branch
from
a9a4c91 to
a9cb858
Compare
| sort.Slice(b, func(i, j int) bool { | ||
| return b[i].less(b[j]) | ||
| }) | ||
| return reflect.DeepEqual(a, b) |
There was a problem hiding this comment.
parent doesn't need to be equal and checked. (See LabelledCell in C++)
4 participants
Adds the methods
IntersectingLabelsandVisitIntersectingCellsas well as the testing.