harsha2010 / magellan

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

Corrected BoundingBox.relate Within case #206

Closed zebehringer closed 6 years ago

zebehringer commented 6 years ago

use [new] Polygon:line contains test instead of intersects optimized the code a little to avoid extra intersection test

codecov-io commented 6 years ago

Codecov Report

Merging #206 into master will increase coverage by 0.07%. The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #206      +/-   ##
==========================================
+ Coverage      90%   90.07%   +0.07%     
==========================================
  Files          52       52              
  Lines        1790     1794       +4     
  Branches      129      125       -4     
==========================================
+ Hits         1611     1616       +5     
+ Misses        179      178       -1
Impacted Files Coverage Δ
src/main/scala/magellan/Polygon.scala 88% <100%> (+0.24%) :arrow_up:
src/main/scala/magellan/BoundingBox.scala 93.61% <83.33%> (+0.13%) :arrow_up:
src/main/scala/magellan/Shape.scala 55% <0%> (+2.5%) :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...ce0c864. Read the comment docs.

harsha2010 commented 6 years ago

@zebehringer as discussed offline, thanks a lot for catching this performance regression. I will take your PR and add some tests to it to capture the regression and then merge this PR in today

harsha2010 commented 6 years ago

@zebehringer I am closing this, I have created a PR #207 based off this which adds some tests and missing functionality as well. Please review it when you get a chance and if possible test it out Thanks for noticing this issue!