ewestern / geos

This is a Haskell binding to Geos, the open-source geometry library
MIT License
13 stars 9 forks source link

Contains test case #13

Closed ewestern closed 7 years ago

jamiecook commented 7 years ago

eek - that's a bit scary - I count about 60 uses of withGeos in the library. And it seems that it's the usage of withGeos that brings in the withForeignPtr call.

Would that mean that any usage of withGeos should follow a strict "don't expect anything from this block to exist later unless you clone it" mentality?

jamiecook commented 7 years ago

So this

image

Would be fine because it uses the underlying ptr as the input to another libgeos method which will return it's own ptr (wrapped in the Geos monad)

But something like

image

Might be a problem as it just uses that ptr in the construction of another Geos object....

Or am i not understanding correctly (getting my head around the various monads and callbacks is still a challenge for me)

ewestern commented 7 years ago

That's right. I think the core problem was making the constructGeometry and createCoordinateSequence instance methods run in the Geos monad rather than IO. Introducing those functions caused me to change some of the functions from the correct behavior to the incorrect behavior. (In your example, if constructGeometry were called within the withCoordinateSequence block, it would have been fine).

I'm going to go ahead and merge this branch in now since it has no conflicts, and then we can make a separate PR for outstanding memory issues.

jamiecook commented 7 years ago

also eek - i didn't actually expect you to merge that holus bolus - I put some crap in there that probably shouldn't be part of the library.

Namely it now depends on cassava (Data.CSV).... I had a crazy idea that it would be good to include parsers for the various interchange formats... I've recently written Cassava, Aeson and Postgres-Simple instances of To/FromX which I thought might be good to include as an additional module.... but that would puts the full set of those packages into the dependency tree for geos, which I'm now thinking is too heavy-weight for a gen purpose library like Geos - thoughts?

Also I added big sample data (points.wkb and polygons.wkb) which may not be something you want to have in the core git repo (sorry).

There is a bit of duplication (ensurePoint, readLotsOfHex, etc) which I will remove in a separate PR.