-
Notifications
You must be signed in to change notification settings - Fork 215
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
Higher-order function schemas #770
Comments
Hi. Just tested, the instrumentation is not recursive at the moment. Should be fixable. here's a hand-rolled recursion: (((m/-instrument
{:schema [:=>
[:cat :int]
[:=> [:cat :int] :int]]}
(fn [x]
(m/-instrument
{:schema [:=> [:cat :int] :int]}
(fn [y] (+ x y)))))
1) 2)
; 3 (((m/-instrument
{:schema [:=>
[:cat :int]
[:=> [:cat :int] :int]]}
(fn [x]
(m/-instrument
{:schema [:=> [:cat :int] :int]}
(fn [y] (str (+ x y))))))
1) 2)
;Execution error (ExceptionInfo) at malli.core/-fail! (core.cljc:136).
;:malli.core/invalid-output {:output :int, :value "3", :args [2], :schema [:=> [:cat :int] :int]} |
so, the instrumentation should look for all child schemas and instrument those too. There is
Maybe start with option 1? |
How about using options to opt-in? (defn adder
{:malli/schema
[:=> {:recur true}
[:cat :int]
[:=> [:cat :int] :int]]}
[x]
(partial + x)) |
That's a good option. One more place to do this would be: (defn adder
{:malli/schema
[:=>
[:cat :int]
[:=> [:cat :int] :int]]
:malli/recur true}
[x]
(partial + x)) |
Would you be interested in doing a PR for this? I think the logic would go into |
I think you proposal of using |
Generator already recur (as expected): (((mg/generate
[:=>
[:cat :int]
[:=> [:cat :int] :int]]) 1) "2")
;Execution error (ExceptionInfo) at malli.core/-fail! (core.cljc:136).
;:malli.core/invalid-input {:input [:cat :int], :args ["2"], :schema [:=> [:cat :int] :int]} |
I can give it a shot
Thanks
Having second thoughts on that, because if someone bothers writing a higher-order function schema instead of justing using It's the effects that have to be controlled, perhaps as an option to |
Supporting the initial example in @@ -2481,9 +2481,10 @@
(report ::invalid-input {:input input, :args args, :schema schema})))
(let [value (apply f args)]
(when wrap-output
- (when-not (validate-output value)
- (report ::invalid-output {:output output, :value value, :args args, :schema schema})))
- value))))
+ (cond
+ (not (validate-output value)) (report ::invalid-output {:output output, :value value, :args args, :schema schema})
+ (= :=> (type output)) (-instrument (assoc props :schema output) value options)
+ :else value))))))
:function (let [arity->info (->> (children schema)
(map (fn [s] (assoc (-function-info s) :f (-instrument (assoc props :schema s) f options))))
(-group-by-arity!)) At this point, ((((mi/-instrument
{:schema [:=>
[:cat :int]
[:=>
[:cat :int]
[:=>
[:cat :int]
:int]]]}
(fn [x]
(fn [y]
(fn [z]
(+ x y z)))))
1) 2) :_)
But in this example the return value is the fn we need to instrument so it's basically the simplest recursion scheme. Things get more complicated if, for example, we consider wrapping the fn inputs. Now we need to: go through the args – along with their input schema – and instrument all the If we could make this process more generic and instead recursively traverse the structure (the args and return value) – along with their input schemas (the input or output) – then we would be able to instrument high-order functions anywhere in the inputs or outputs, which would be pretty cool. This sounds a lot like what transformers do, but I could not manage for the handlers in |
Sadly, transformers don't receive both schema & value atm. Did a spike on adding schema to transforming functions some time ago, but was a big change and did not finish. As the simple solutions looks really simple, I propose let's merge that in, better than current, just needs documentation that it is not recursive. What do you think? Also, function can be |
Judging by the repl transcript below, higher-order function schemas are not currently supported.
Is it feasible in the current schema-function/instrumentation design?
The text was updated successfully, but these errors were encountered: