Update kwargs fallback rules after RuleConfig rewrite #372
Conversation
Codecov Report
@@ Coverage Diff @@
## master #372 +/- ##
==========================================
+ Coverage 88.86% 89.46% +0.59%
==========================================
Files 14 14
Lines 485 560 +75
==========================================
+ Hits 431 501 +70
- Misses 54 59 +5
Continue to review full report at Codecov.
|
Keno
force-pushed
the
kf/updatespecialrules
branch
2 times, most recently
from
d64e504 to
dc52994
Compare
src/rules.jl
Outdated
| See also: [`rrule`](@ref), [`@scalar_rule`](@ref), [`RuleConfig`](@ref) | ||
| """ | ||
| frule(::Any, ::Any, ::Vararg{Any}; kwargs...) = nothing | ||
| frule(::Any, ::Any, ::Vararg{Any}) = nothing |
There was a problem hiding this comment.
Is there a reason we don't just do
| frule(::Any, ::Any, ::Vararg{Any}) = nothing | |
| frule(::Vararg{Any}) = nothing |
?
There was a problem hiding this comment.
yes, we require at least 2 arguments.
The minimum number of arguments a function can have is zero.
Which has the case frule(Tuple(), f).
There was a problem hiding this comment.
Either way is fine. The system doesn't care, but this makes it explicit that at least two arguments are expected, so you get a method error otherwise. I guess by the same reasoning, I should put back the two arguments below.
src/rules.jl
Outdated
| # still has to manually analyze). Manually declare this method with an | ||
| # explicitly empty body to save the compiler that work. | ||
| const frule_kwfunc = Core.kwftype(typeof(frule)).instance | ||
| (::typeof(frule_kwfunc))(::Any, ::typeof(frule), ::Any, ::Vararg{Any}) = nothing |
There was a problem hiding this comment.
I guess either having the two Anys
| (::typeof(frule_kwfunc))(::Any, ::typeof(frule), ::Any, ::Vararg{Any}) = nothing | |
| (::typeof(frule_kwfunc))(::Any, ::typeof(frule), ::Any, ::Any, ::Vararg{Any}) = nothing |
or pack them all into Vararg
| (::typeof(frule_kwfunc))(::Any, ::typeof(frule), ::Any, ::Vararg{Any}) = nothing | |
| (::typeof(frule_kwfunc))(::Any, ::typeof(frule), ::Vararg{Any}) = nothing |
right?
There was a problem hiding this comment.
Yeah, let me make it consistent.
Keno
force-pushed
the
kf/updatespecialrules
branch
from
dc52994 to
7c40e72
Compare
4 participants
Fixes #368