Use SimpleUnPack.jl instead of UnPack.jl #2288
Conversation
| @@ -70,7 +70,7 @@ using PrecompileTools, Reexport | |||
|
|
|||
| import Graphs: SimpleDiGraph, add_edge!, incidence_matrix | |||
|
|
|||
| @reexport using UnPack | |||
| @reexport using SimpleUnPack | |||
There was a problem hiding this comment.
I'm a bit surprised about this re-export, is this actually needed? Or could it be limited to
| @reexport using SimpleUnPack | |
| @reexport using SimpleUnPack: @unpack |
?
|
We cannot do this because we actually overload the |
Yes, I saw that and removed it in the PR - but it seems it's just forwarded to |
|
@YingboMa or did you mean something else? Is this |
|
No, |
| @@ -1800,10 +1800,6 @@ function compose(syss...; name = nameof(first(syss))) | |||
| end | |||
| Base.:(∘)(sys1::AbstractSystem, sys2::AbstractSystem) = compose(sys1, sys2) | |||
|
|
|||
| function UnPack.unpack(sys::ModelingToolkit.AbstractSystem, ::Val{p}) where {p} | |||
| getproperty(sys, p; namespace = false) | |||
There was a problem hiding this comment.
unpack changes the default. It doesn't just forwards.
There was a problem hiding this comment.
Ah, I missed the namespace keyword argument - I guess that's the problem here?
|
Even if #2288 (comment) is problematic and this overload can't be removed, I think the PR should be reconsidered. Many uses of ModelingToolkit.jl/src/systems/systems.jl Line 25 in 34d1853 |
In this PR, I propose to switch from UnPack to SimpleUnPack, for reasons described in its README: https://github.com/devmotion/SimpleUnPack.jl