harsha2010 / magellan

Geo Spatial Data Analytics on Spark
Apache License 2.0
533 stars 149 forks source link

Fix relate bug #207

Closed halfabrane closed 6 years ago

halfabrane commented 6 years ago

Previously Polygon#intersects(Line) used to be a strict intersection check With 62910c30, this behavior changed. This breaks BoundingBox#relate which now always returns Intersects even when it should return Within, creating a performance regression

This PR adds tests to BoundingBox#relate which if they existed would have caught the issue It also introduces a strict intersection check it is based off a PR from @zebehringer who first noticed this regression (thanks Zack!)

@zebehringer for review

codecov-io commented 6 years ago

Codecov Report

Merging #207 into master will increase coverage by 0.16%. The diff coverage is 90.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #207      +/-   ##
==========================================
+ Coverage      90%   90.16%   +0.16%     
==========================================
  Files          52       52              
  Lines        1790     1789       -1     
  Branches      129      124       -5     
==========================================
+ Hits         1611     1613       +2     
+ Misses        179      176       -3
Impacted Files Coverage Δ
src/main/scala/magellan/BoundingBox.scala 95.65% <100%> (+2.17%) :arrow_up:
src/main/scala/magellan/Polygon.scala 87.58% <100%> (-0.17%) :arrow_down:
src/main/scala/magellan/PolyLine.scala 89.47% <100%> (ø) :arrow_up:
src/main/scala/magellan/Shape.scala 58.53% <80%> (+6.03%) :arrow_up:
src/main/scala/magellan/catalyst/SpatialJoin.scala 95.23% <0%> (-2.39%) :arrow_down:
...in/scala/org/apache/spark/sql/types/PointUDT.scala 100% <0%> (+3.12%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a226690...63275a3. Read the comment docs.

harsha2010 commented 6 years ago

Thanks @zebehringer