Add remake() for reconstructing immutable structs #87
Conversation
|
|
||
|
|
||
| """ | ||
| @add_kwonly function_definition |
There was a problem hiding this comment.
I feel like this could even get its own package. That's quite a handy macro.
| Re-construct `thing` with new field values specified by the keyword | ||
| arguments. | ||
| """ | ||
| function remake(thing; kwargs...) |
There was a problem hiding this comment.
Oops. I just comment it on #87 (review)
| @test prob1.tspan == prob2.tspan | ||
| @test prob1.jac_prototype === prob2.jac_prototype | ||
| @test prob1.callback === prob2.callback | ||
| @test prob1.mass_matrix === prob2.mass_matrix |
There was a problem hiding this comment.
Probably want to add some inference tests. Some might fail, in which case just @test_broken them so we can keep track of it for v0.7.
| T | ||
| else | ||
| # Assume that T wants isinplace | ||
| T{isinplace(thing)} |
There was a problem hiding this comment.
I don't like that it assumes the constructor has the iip type parameter but it should be fine as long as remake is used for types from DiffEqBase. Note that I can't dispatch on DEProblem to avoid this since using it means that remake won't work with SplitFunction etc.
|
That's a very clean solution. You should consider making |
Codecov Report
@@ Coverage Diff @@
## master #87 +/- ##
==========================================
+ Coverage 19.4% 21.91% +2.51%
==========================================
Files 34 34
Lines 1134 1182 +48
==========================================
+ Hits 220 259 +39
- Misses 914 923 +9
Continue to review full report at Codecov.
|
|
Thanks :) Well, it needs a bit of help from |
|
Sounds good. Then just some inference checks and it'll be done. If inference is really bad then it may need to be an |
|
BTW, I think current |
Yes, remove it. It's undocumented and so it's fine. Make sure we ping whoever added it. I think it was @BeastyBlacksmith ? Check blame. |
|
thanks for letting me know. Is there any documentation string I can read? I am super stressed at the moment to read all git diff. |
|
|
|
okay great |
|
@ChrisRackauckas So I wrote the generated version (see #89) but |
|
It looks like this is it: JuliaLang/julia#25918 Example: type Spam{T}
Spam(x::T) where T = new{T}()
Spam(; x::T = nothing) where T = new{T}()
end
using Base.Test
macro test_nothrow(ex)
quote
@test begin
$(esc(ex))
true
end
end
end
@testset "Spam" begin
@test_nothrow @inferred Spam(nothing)
@test_nothrow @inferred Spam() # it fails
# @test_nothrow @inferred Spam(x="x")
end |
|
Ugh, keyword arguments on v0.6.2 don't even infer the default usage? They used to. I moved the last args to keyword arguments and it worked... okay then whatever. Mark as |
|
Alright, then let's do this for now and leave fixing the inference for v0.7 when kwargs are fixed. |
|
Thank you for the merge! |
closes SciML/DifferentialEquations.jl#230