remove duplicate code for collocation #180
Conversation
| kernel_function, | ||
| tpoints) | ||
| if kernel isa Symbol | ||
| @warn "Passing kernels as Symbols will be deprecated" |
There was a problem hiding this comment.
You can use @deprecate here
There was a problem hiding this comment.
I wanted to do it like that at first, but I ran into issues because Julia cannot dispatch on a keyword argument, i.e. it is impossible to have two methods, one with kernel restricted to Symbol types and one with kernel restricted to CollocationKernel, and then @deprecate the Symbol method.
There was a problem hiding this comment.
I don't think you can @depreacate here, it needs this special handling. But I also don't think we should be merging deprecations to this repo: it's somewhat in freeze mode because its interface does not extend well to more cases. In that case, major dependency changes and deprecations for an old repo not getting any major benefits is not a great feel. I think this PR should just hold until there's a good reason found for it.
There was a problem hiding this comment.
Yeah without kwarg handling it's not obvious, I think the way to do it with @deprecate would be
(though it's not relevant anymore since we won't go ahead with the PR, but it might be useful later)
my_func(args...; kwarg_to_deprecate::Union{T1,T2}, kwargs...) = my_func(args..., kwarg_to_deprecate; kwargs...)
my_func(args..., kwarg_to_deprecate::T1; kwargs....) = do something...
@deprecate my_func(args..., kwarg_to_deprecate::T2; kwargs...) = do something....
DiffEqFlux has copy pastes of some of the functions here.
They should be removed in one of the two.
SciML/DiffEqFlux.jl#773
I would also suggest deprecating passing the kernel as a symbol, to make it more uniform with the rest of the SciML interfaces, e.g.
Tsit5()is used, not:Tsit5.