Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Getting error while finding difference between a multipolygon and a polygon #721

Closed
amishas157 opened this issue May 8, 2017 · 15 comments
Closed
Assignees
Labels
Milestone

Comments

@amishas157
Copy link

I am getting following error while finding the difference betweeen a polygon and a multipolygon.

/Users/AmishaSingla/Documents/osm-adiff-compare/node_modules/@mapbox/osm-compare/node_modules/@turf/difference/node_modules/jsts/dist/jsts.min.js:14
ln.checkValid(this.edgeList.getEdges()),this.graph.addEdges(this.edgeList.getEdges()),this.computeLabelling(),this.labelIncompleteNodes(),this.findResultAreaEdges(t),this.cancelDuplicateResultEdges();var n=new Sn(this.geomFact);n.add(this.graph),this.resultPolyList=n.getPolygons();var i=new wn(this,this.geomFact,this.ptLocator);this.resultLineList=i.build(t);var r=new Ln(this,this.geomFact,this.ptLocator);this.resultPointList=r.build(t),this.resultGeom=this.computeGeometry(this.resultPointList,this.resultLineList,this.resultPolyList,t)},labelIncompleteNode:function(t,e){var n=this.ptLocator.locate(t.getCoordinate(),this.arg[e].getGeometry());t.getLabel().setLocation(e,n)},copyPoints:function(t){for(var e=this.arg[t].getNodeIterator();e.hasNext();){var n=e.next(),i=this.graph.addNode(n.getCoordinate());i.setLabel(t,n.getLabel().getLocation(t))}},findResultAreaEd

The polygon and multipolygon are following:

Source Code:
https://jsfiddle.net/tboq1cv5/

Not sure if the input to difference method is right or not.

cc @rowanwins @DenisCarriere

@rowanwins
Copy link
Member

Hi @amishas157

This is likely due to the same issue previously around jsts not supporting multipolygons very well.

Also as an FYI - in jsfiddle you can add an external resource like the minified version of turf with the url https://npmcdn.com/@turf/turf/turf.min.js. then rather the require failing you can just say turf.difference(feature, feature2). Check out this example and open the console to see the result, In this case I just took one part of your multipoly and it works fine, so yeah Im pretty sure its the multipoly thing.

Cheers

@amishas157
Copy link
Author

Whoops cool. Thank for the turfy information. : ) Will try it sure.

@DenisCarriere DenisCarriere added this to the 4.4.0 milestone May 9, 2017
@DenisCarriere
Copy link
Member

Looks like @turf/difference doesn't support MultiPolygon, however now that we've got a spiffy flattenEach method we could iterate over each flattened Polygon and apply the same jsts difference operations. This shouldn't be to hard to include, @amishas157 let me look into this.

@DenisCarriere DenisCarriere self-assigned this May 9, 2017
@DenisCarriere
Copy link
Member

DenisCarriere commented May 9, 2017

@amishas157 Looks like the 2nd Multi-Polygon is really a LineString (seems like the issue is coming from jsts).

Solution (quick fix):

We could remove empty polygons prior to performing the difference operations.

image

DenisCarriere added a commit that referenced this issue May 9, 2017
- Major refactoring to `index.js` & `test.js`
- Update JSDocs & Typescript definition to support MultiPolygon
#721
DenisCarriere added a commit that referenced this issue May 10, 2017
* Update fixtures from #644

* Fix jsts empty (Multi)Polygon error
- Major refactoring to `index.js` & `test.js`
- Update JSDocs & Typescript definition to support MultiPolygon
#721
@DenisCarriere DenisCarriere modified the milestones: 4.3.0, 4.4.0 May 10, 2017
@amishas157
Copy link
Author

@DenisCarriere Little confused, do we need to use flattenEach method or do any preprocessing before applying difference operation?

@DenisCarriere
Copy link
Member

No, the issue wasn't the MultiPolygon, it was related to empty polygons.
In the next release you won't have to change anything in your code, the fix excludes those empty polygon geometries.

@DenisCarriere
Copy link
Member

@amishas157 Just released a patch release v4.2.1 @turf/difference, try it out now and let me know if this fixed the issue for you.

Note: This won't be fixed in the main TurfJS library, changes only apply in @turf/difference.

@amishas157
Copy link
Author

Wohooo 🎉 On it.

@amishas157
Copy link
Author

Hey @DenisCarriere 👋 I tried running this code sinppet https://jsfiddle.net/tboq1cv5/ but looks like it is giving the error. Can you confirm once?

@DenisCarriere
Copy link
Member

@amishas157 Looks like patch release didn't include the newest changes (must of published v4.2.1 on a different branch 🤦‍♂️ ).

Ok I've published another patch release v4.2.2 and this one I can confirm does work! 👍

Lesson learned.. don't publish npm releases before going to bed at 1:30am 🛏

@amishas157
Copy link
Author

Ok I've published another patch release v4.2.2 and this one I can confirm does work! 👍

🙇‍♀️

Lesson learned.. don't publish npm releases before going to bed at 1:30am 🛏

Yes, That time is meant for sleeping 😴 and work 🙅‍♂️ .

@amishas157
Copy link
Author

@DenisCarriere , oops I think we need to also publish a new package for turf-invariant, which has got getGeom method and use that in turrf-difference, coz right now i see this error:

/Users/AmishaSingla/Documents/experiments/node_modules/@turf/difference/index.js:52
    var geom1 = getGeom(polygon1);
                ^

TypeError: getGeom is not a function
    at module.exports (/Users/AmishaSingla/Documents/experiments/node_modules/@turf/difference/index.js:52:17)
    at Object.<anonymous> (/Users/AmishaSingla/Documents/experiments/turf-experiment.js:316:13)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)
    at startup (node.js:139:18)
    at node.js:968:3

@DenisCarriere
Copy link
Member

Strange that I didn't get that error, might be my local environnement. Ok let's wait and test this again until I publish all the other repos

@DenisCarriere
Copy link
Member

DenisCarriere commented May 12, 2017

@amishas157 Just published v4.3.0, let me know if this issue is still happening on other data.

I used @rowanwins's approach and everything seems to work: https://jsfiddle.net/yjctqtc6/

Feel free to close if it's resolve 👍

@amishas157
Copy link
Author

Awesome, this works 💯 . Thanks @DenisCarriere :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants