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

Clojurescript support #69

Open
willcohen opened this issue Apr 24, 2020 · 8 comments
Open

Clojurescript support #69

willcohen opened this issue Apr 24, 2020 · 8 comments

Comments

@willcohen
Copy link
Contributor

willcohen commented Apr 24, 2020

Before I embark on what would be a very large pull request, I thought I'd first check in with you all on whether it makes sense. Today's Clojurescript release contains functionality which, I think, finally enables the ability to port geo to Clojurescript.

There are already javascript analogues to most of the functionality in here on the java side: jsts, proj4js, h3-js, and so on. I've already gotten proj4js mostly working, transforming the full list of projections already available to geo through proj4j. All of the interop issues between java spatial libraries apply just as much to javascript, so I think this would be similarly valuable, and having the syntax work similarly between platforms seems very very useful. As I keep working on libraries that build on top of geo, it'd open all of them up to working cross-platform too, with (hopefully) fairly low cljc syntax needed later on.

This would, though, be a big change to geo. It wouldn't change any of the existing library, but I'd need to add a bunch of .cljc logic whenever things drop down to java, which is often, or whenever functionality between java and javascript aren't quite the same. Similarly, this would probably require moving away from midje and to clojure.test, which allows for cross compatibility with cljs.test.

If this seems within the scope of this library, how would it sound for me to start working on a PR in a new branch?

@worace

@worace
Copy link
Contributor

worace commented Apr 24, 2020

Ha this is an interesting idea and I have to admit it had never really occurred to me to try to use this under cljs. I'm not really a clojurescript user myself but I'm sympathetic to trying to support that part of the ecosystem as long as it's not a totally unreasonable maintenance burden.

Off the cuff, here are a few questions that stand out to me:

  • Is spatial4j a problem? That's the main one i see missing from your list
  • How does the H3 one actually work? I thought the only H3 implementation was in C (I am somewhat familiar with this because I spent a decent amount of time trying to port it to rust at one point). Did they JS-ify it with emscripten or something?
  • What would the code actually look like for all of this? At the end of the day a huge chunk of this library is just interop between all these java libs. So the obvious concern would be that we just end up having to duplicate 60-70% of the code, at which point we have to ask if it's even the same library any more. What would even go in the shared .cljc files? Just the protocol definitions and whatever stuff we can define in pure clojure? And then we'd have dual impls for all the protocols for the respective native libraries?

@willcohen
Copy link
Contributor Author

Those are all good questions!

It's true, I haven't found an exact replacement for spatial4j, meaning that things like the pole wrap and great circle won't exist, for now, in clojurescript. That said, lots of what spatial4j does work in JTS too, and an initial issue was one of licensing. The protocols in geo.spatial need to be ported, but I think they just won't be extended to any javascript types. In the cases where functions more universally depend on spatial4j, some workarounds may be needed. I haven't done enough research yet to find what the most common geohash implementation is in js-land, but if there's a relatively universal one, that one could get included too.

For h3, that's exactly right -- it transpiles with emscripten.

Finally, I'm still working on the code organization a little bit. I think most files that are currently .clj can actually just be .cljc. In many cases, only one or two lines in each function needs to have alternative implementations, almost always centered on the actual java dropdown calls or type hints. This would have the substantial readability benefit of keeping the similar implementations right next to each other, so it's much easier to maintain parity between the two. Given how close jsts, h3-js, and proj4js (minus the nonexistence of the CoordinateReferenceSystem object, which can be sidestepped with a new deftype in geo.crs), the overall processes should work pretty similarly. As I work on porting, I'll see if there's ways to pull out the java/js stuff to help keep the pure clojure stuff more unified.

The one exception might be geo.jts. Simply because jsts is a different name, my recommendation would to be to maintain jts.clj as a clojure-only namespace, and jsts.cljs as a clojurescript-only namespace. In this case, the two files would be very similar, but it seems funny for people to have to include [geo.jts :as jsts] or something downstream. That said, I'd probably suggest keeping the jts testing as cljc, so that we can enforce parity between the two namespaces with one test set.

@zackteo
Copy link
Contributor

zackteo commented Aug 19, 2020

@willcohen Am curious but how has this been going? :o

@willcohen
Copy link
Contributor Author

I’ve got it mostly working internally but it’s a little messy. I’ll try to get a partial pull request up soon for initial review! Sorry for the delay — other things got in the way!

@zackteo
Copy link
Contributor

zackteo commented Aug 19, 2020

Thank you! Yeah no worries, I'm just exploring Clojure's GIS support, as I prepare for an upcoming project that deals with spatial data.

@willcohen
Copy link
Contributor Author

@worace It's been a slow process but this port is coming along steadily. Right now the WIP version of it lives at https://github.com/willcohen/geo/tree/clojurescript. Should I just keep it there until it's ready for a PR?

@worace
Copy link
Contributor

worace commented Oct 19, 2020

That seems fine to me. I think the most useful thing would be to evaluate how the diff is looking and determine what this is going to look like from a maintenance perspective. Basically is it just going to be too complicated to be worth it. Might be worth doing that before you get too deep into painstaking conversion work.

@willcohen
Copy link
Contributor Author

I'd say the diff is 80% there at this point -- my main tasks remaining, I think, are to get the test suite fully converted and passing (I should have JTS/JSTS tests done in the next few days), and to finish porting the h3 namespace. Generally speaking, I'd say it won't get any less maintainable than it currently is.

For what it's worth, I think it is ABSOLUTELY worth it since it'll be an idiomatically parallel version of GIS for the frontend, but we should definitely talk once it stops being only partially done! Most of the namespaces are pure .cljc copies with some parts conditionally rewritten for interop. The biggest one has been JTS and JSTS -- I ended up breaking it out into two new namespaces called geo.impl.jts and geo.impl.jsts, since it turns out that there's some deep interop stuff that's needed for both geo.jts AND geo.crs, and that way both of those namespaces can depend on the impl namespaces without making anything circular.

With the exception of spatial4j, pretty much everything has a parallel. I did rewrite a clojure-native version of the vincenty distance calculator, so that that works in clj as well as cljs, since distance is used in a bunch of other functions.

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

No branches or pull requests

3 participants