Skip to content

Fix Monad instance for PolygonalSummaryResult - #3221

Merged
echeipesh merged 2 commits into
locationtech:masterfrom
echeipesh:fix/polygonalsummaryresult-monad
Apr 7, 2020
Merged

Fix Monad instance for PolygonalSummaryResult#3221
echeipesh merged 2 commits into
locationtech:masterfrom
echeipesh:fix/polygonalsummaryresult-monad

Conversation

@echeipesh

@echeipesh echeipesh commented Apr 7, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR fixes the implementation of the Monad instance to avoid self-recursion

Problem

scala> val s: PolygonalSummaryResult[Int] = Summary(3)
s: geotrellis.raster.summary.polygonal.PolygonalSummaryResult[Int] = Summary(3)

scala> s.map(x => x.toString)
java.lang.StackOverflowError
  at cats.Monad$$Lambda$6830/427965418.<init>(Unknown Source)
  at cats.Monad$$Lambda$6830/427965418.get$Lambda(Unknown Source)
  at cats.Monad.map(Monad.scala:16)
  at cats.Monad.map$(Monad.scala:14)
  at geotrellis.raster.summary.polygonal.PolygonalSummaryResult$$anon$1.map(PolygonalSummaryResult.scala:42)
  at geotrellis.raster.summary.polygonal.PolygonalSummaryResult$$anon$1.flatMap(PolygonalSummaryResult.scala:45)
  at geotrellis.raster.summary.polygonal.PolygonalSummaryResult$$anon$1.flatMap(PolygonalSummaryResult.scala:42)
  at cats.Monad.map(Monad.scala:16)
  at cats.Monad.map$(Monad.scala:14)
 ...

Fixed

scala> import geotrellis.raster.summary.polygonal._
import geotrellis.raster.summary.polygonal._

scala> import cats.syntax.functor._
import cats.syntax.functor._

scala> (Summary(3): PolygonalSummaryResult[Int]).map( x => "asdf")
res2: geotrellis.raster.summary.polygonal.PolygonalSummaryResult[String] = Summary(asdf)

Note that the Summary instance has to be upcast to PolygonalSummaryResult in order for the provided Monad instance to be discovered. This looks weird in console but in practice its not much of an issue because the point of this ADT is that you don't know if the result will be Summary or NoIntersection. That is all polygonal summary functions actually have return type of PoloygonalSummaryResult[A]

echeipesh added 2 commits April 7, 2020 12:40
@echeipesh
Fix Monad instance for PolygonalSummaryResult
9e0ff8c 
Avoids StackOverflow through unterminated self-recursion
@echeipesh
Update changelog [skip-ci]
dfead7e
@echeipesh
echeipesh requested a review from pomadchin April 7, 2020 16:50
pomadchin
pomadchin approved these changes Apr 7, 2020
View reviewed changes

@pomadchin pomadchin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@echeipesh
echeipesh merged commit 2a7531d into locationtech:master Apr 7, 2020
@pomadchin
pomadchin deleted the fix/polygonalsummaryresult-monad branch August 15, 2020 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pomadchin pomadchin pomadchin approved these changes

Assignees

@echeipesh echeipesh

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@echeipesh @pomadchin