aysylu / loom

Graph library for Clojure. Mailing list https://groups.google.com/forum/#!forum/loom-clj
http://aysy.lu/loom/
887 stars 108 forks source link

Make loom Clojure[Script] portable #91

Closed cemerick closed 7 years ago

cemerick commented 7 years ago

(This PR supersedes PR https://github.com/aysylu/loom/pull/76, and when merged should close #45.)

While I'm using a small subset of loom's functionality, I do have the results of this work running in the browser in a nontrivial application -- so I think it's in decent shape, beyond the project's own tests.

If you'd like to try it out in a ClojureScript app/environment, just add [org.clojars.cemerick/loom "0.6.1-SNAPSHOT"] to your dependencies.

Bits that remain Clojure-only:

Aside from the above caveats re: portability, and the extended documentation of the extend hack/solution in loom.cljs, I'd wanted to note that I ended up needing to tweak a number of tests to account for different ordering implementation details in ClojureScript (at the moment!) vs. Clojure. See loom.test.alg/bipartite-test for an example; there are many possible solutions for many of the algorithms in loom, so simple equality assertions aren't sufficient to really establish correctness. Representing expected values as sets (and transforming results similarly) can help, but this will be a persistent problem given the existing test approach.

Finally, please note the new :aliases in project.clj. A quick skimming of [doo]()'s readme would be good if anyone plans on running the test suite in ClojureScript in an interactive setting. (tl;dr: lein do clean, doo node)

I hope this meets expectations. Let me know what your questions are.

cemerick commented 7 years ago

Just now noticed that the CI failed, turns out ClojureScript doesn't like OpenJDK 6 (7+ is required). PR now includes an updated travis config, swapping the openjdk6 target for JDK8.

pataprogramming commented 7 years ago

Thanks very much for this excellent pull request! I'll begin reviewing this so we can get it merged and a new release cut

viebel commented 7 years ago

@pataprogramming @cemerick it works like a charm inside KLIPSE. Take a look at this interactive gist

viebel commented 7 years ago

Once this is merged, you can make the documentation interactive using codox klipse theme.

dustingetz commented 7 years ago

Thanks @cemerick for this work. Fork is working great for me.

viebel commented 7 years ago

@dustingetz @cemerick's code work fine in self-host cljs also. Live demo in KLIPSE: http://app.klipse.tech/?eval_only=1&cljs_in.gist=viebel/6631844e78002a5c6098d5ee03562f36&external-libs=[%22https://raw.githubusercontent.com/cemerick/loom/cljs/src/%22,%22https://raw.githubusercontent.com/tailrecursion/cljs-priority-map/master/src/cljs/%22]

It should be super simple now to make the documentation interactive using codox klipse theme.

cemerick commented 7 years ago

:wave: Just saying hi, thought I'd check in and see if there's any further help needed here. I'm happy enough using my own build as necessary, but if there's tweaking to be done here, I'd love to take care of it while the work is still somewhat front-of-mind.

aysylu commented 7 years ago

Hey @cemerick, sorry about the delay and thanks for the great PR! Either @pataprogramming or I will respond to this PR this weekend.

cemerick commented 7 years ago

Please, no apologies. I know the OSS drill, definitely don't want to change any priorities, etc. Enjoy this weekend long before you look at this PR. :heart:

pataprogramming commented 7 years ago

I've finally been able to sit down and spend the detailed time with this pull request that it deserves.

First: Abject apologies to @cemerick and the community of Loom users that it's taken this long!

Second: Thank you, @cemerick, for a brilliantly clean and well-written pull request. My own use of ClojureScript has been relatively limited, and I've learned quite a number of useful things on how to structure projects while walking through your changes.

Third: I'm accepting this pull request as submitted. If any Loom users have additional feedback on the design or run into problems, please submit them as issues. If there's no significant pushback, we'll cut a new release soon.