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

Add CoordinateSequenceFilter to geom.js #402

Merged
merged 1 commit into from
Apr 27, 2020

Conversation

willcohen
Copy link
Contributor

I'm working on making geo cross-compatible with Clojurescript in addition to Clojure, which means integrating JSTS alongside JTS, and ran into the same issue as #382.

geo performs all of its reprojection/transformation by implementing CoordinateSequenceFilter in JTS, and JTS Javadocs seem to suggest that this is the preferred method for doing that kind of transformation.

By creating a function that that implements the five methods of CoordinateSequenceFilter (and correctly returns the class to be able to satisfy hasInterface, assuming it's exportable from geom.js), transformations of all geometries seem to be working correctly using the same process here as in JTS.

@bjornharrtell

@codecov
Copy link

codecov bot commented Apr 27, 2020

Codecov Report

Merging #402 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #402   +/-   ##
=======================================
  Coverage   62.61%   62.61%           
=======================================
  Files         279      279           
  Lines       14245    14245           
=======================================
  Hits         8920     8920           
  Misses       5325     5325           

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 83b5aee...b64a47c. Read the comment docs.

@bjornharrtell
Copy link
Owner

bjornharrtell commented Apr 27, 2020

That's a funky use of JSTS. :) I would like to support your use case and I'm fine with the change in this PR but I'm not fully understanding if you are also affected what was observed in #382 about empty interfaces_ which is a bit harder to fix?

@willcohen
Copy link
Contributor Author

willcohen commented Apr 27, 2020

No -- in this case, in the new object that implements the filter interface, hasInterface seems fine with me just returning for interfaces_ a new single item array with[jsts.geom.CoordinateSequenceFilter] as its contents, and the application of the filter proceeds successfully from there.

Separately, thanks for all of your work on this library (and jts2geojson, which we've also been using for quite some time!)

@bjornharrtell
Copy link
Owner

Thanks for the kind words, I'll merge this now and hopefully get a new release out in a not too far future.

@bjornharrtell bjornharrtell merged commit 8e5ee8b into bjornharrtell:master Apr 27, 2020
@willcohen willcohen deleted the csf branch April 27, 2020 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants