Bug: incorrect behavior of `@mlj_model` for vector parameters

[MilesCranmer]

I'm not sure if this is intended, but it seems like you cannot have a default parameter be a vector?

julia> @macroexpand @mlj_model mutable struct MyModel3 <: Deterministic
                  t = [1, 2]
               end
quote
    #= /Users/mcranmer/.julia/packages/MLJModelInterface/gw4ML/src/model_def.jl:212 =#
    begin
        $(Expr(:meta, :doc))
        mutable struct MyModel3 <: Deterministic
            #= REPL[23]:2 =#
            t
            function MyModel3(; t = 1)
...                              #  ^

i.e., it only takes the first parameter here, and thinks the second element is a constraint:

    function MLJModelInterface.clean!(model::MyModel3)
        warning = ""
        if !2
            warning *= "Constraint `2` failed; using default: t=1."
            model.t = 1
        end
        return warning
    end

---

Edit: here is the equivalent with Base.@kwdef:

julia> @macroexpand Base.@kwdef struct MyModel4
           t = [1, 2]
       end
quote
    #= util.jl:589 =#
    begin
        $(Expr(:meta, :doc))
        struct MyModel4
            #= REPL[25]:2 =#
            t
        end
    end
    #= util.jl:590 =#
    MyModel4(; t = [1, 2]) = begin
            #= util.jl:567 =#
            MyModel4(t)
        end
end

[ablaom]

@MilesCranmer

Thanks for reporting and diagnosing. Maybe I'd call this an undocumented limitation, rather than a bug :-).

Unfortunately, we can't just switch to using Base.@kwarg, as the current behaviour also puts a call to clean! in the kw-consructor, as I recall.

Of course the workaround is to just not use the macro, and provide everything manually, as described in the docs.

Unless you have a suggestion for a non-breaking fix here, I suggest tagging the limitation in documentation. It could go in the doc-string and here.

[MilesCranmer]

Thanks for explaining. Is this a fundamental limitation of the syntax used, or is it something that could be fixed in the parsing?

My mention of Base.@kwdef is more to note that it is possible to get it working with the right parser; perhaps the parsing strategy it uses internally could be used instead, as it correctly extracts the vector. I understand it doesn’t account for the extra assertion function in the @mlj_model, but maybe it wouldn’t be so bad to add?

---

It might also be worth it to try something similar to SimpleTraits.jl quadruple colon syntax, as a way of separating normal :: from the one used for the value assertion. But maybe that isn’t the issue here.

[MilesCranmer]

I think this is definitely a bug because:

struct Foo
    x::Vector{Int} = [0, 1, 2]::(true)
end

works, producing default value [0, 1, 2], whereas

struct Foo
    x::Vector{Int} = [0, 1, 2]
end

results in default value 0.

I have a patch drafted and will push a PR soon; the fix is pretty simple.

[MilesCranmer]

Fixed by #175