Closed holyjak closed 4 months ago
Draft b/c it is branched from #55 and should be rebased on master when this gets merged.
We have some inconsistent behaviour here. It's not necessarily a problem to fail with an error because Subtract[5,4,2] fails inside mathematica with an error, but w/- and w/Subtract should probably either both fail (not just return an unnamespaced symbol) or (w/- 5 4 3) should follow the clojure way of doing things and w/Subtract... should fail.
What do you think @holyjak ?
(wl/eval (w/- 5 4 3)) => ERROR
(wl/eval '(w/- 5 4 3)) => ERROR
(wl/eval (w/Subtract 5 4 3)) => (Subtract 5 4 3)
(wl/eval '(w/Subtract 5 4 3)) => (Subtract 5 4 3)
The ERROR above is:
1. Unhandled java.lang.IllegalArgumentException
Can't handle more than 2 arguments
...
It makes sense but I disagree. If you use Subtract then you likely know Wolfram and know to only pass 2 args. If you use w/- then you likely know Clojure and expect this to behave as clj's - and take any number of args. It doesn't, so I make that clear. Notice it could be argued we already are inconsistent, since (w/- 1) works but Subtract[-1] doesn't. Also, detecting / reporting errors from Wolfram sucks...
Fair points, though my feeling is that Subtract (really all functions with the wrong number of arguments ) should still fail with an error (in the future). A slight counterargument to the 'already inconsistent' idea (although it's a question of inconsistent w.r.t. what?) would be that you will basically never get a failure in Mathematica when using '-' because it's a binary operator that will sometimes be interpreted as Subtract and sometimes Minus, whereas you will get an error with Subtract in the stated situation.
Maybe this is something else that could be addressed in #59, i.e. that a 'better defaults' or 'extended' version of wolfram.clj could interpret (w/-...) the clojurey way for usability reasons.
Problem: Clojure devs are used to
-
taking any number of args. But we map that toSubtact
, which only takes two.Fix: Decide whether to use Minus or Subtract based on the number of arguments, and throw if too many. Do this during convert, by adding an experimental support for having a fn as the alias value. Also modify wolfram ns to keep the original symbols (e.g. - for -) so the conversion still is done by convert, plus add docstrings there for these.
Cleanup: rm
(def fn ..)
from the wolfram ns, as we are adding a macro of the same name there.Additional changes:
eval
was now suddenly clojure.core/eval). Revert was the simplest fix.Review tip: review commit by commit