Skip to content

LearnAPI.clone calls the advertised keyword constructor positionally #67

Description

@s-celles

Hello,

While working on a downstream package (OnlineML.jl), I noticed a discrepancy between the constructor trait documentation and the actual implementation of LearnAPI.clone.

The constructor trait documentation requires a keyword constructor and demonstrates reconstruction with:

LearnAPI.constructor(learner)(; named_properties...)

However, LearnAPI.clone currently collects the learner properties and passes them as positional arguments by splatting a NamedTuple without a semicolon:

LearnAPI.constructor(learner)(NamedTuple{names}(new_values)...)

Minimal reproducer

using LearnAPI

Base.@kwdef struct DemoLearner
    rate::Float64 = 0.1
end

LearnAPI.constructor(::DemoLearner) = DemoLearner

LearnAPI.clone(DemoLearner(); rate=0.2)

Expected Behavior

DemoLearner(0.2) should be returned through the documented keyword-constructor contract (e.g., if clone splatted with a semicolon: ; NamedTuple{names}(new_values)...).

Actual Behavior

clone calls DemoLearner(0.2) positionally. A learner that intentionally provides only the documented keyword constructor (like the one generated by Base.@kwdef without a custom positional fallback) raises a MethodError.

Additional Context

Currently, downstream learners with properties must provide an additional positional constructor solely for compatibility with LearnAPI.clone.

Fixing this by changing clone to splat the named tuple as keywords (;) would align the code with the documentation. However, please note that changing clone upstream may affect existing learners that have already implemented (or only implemented) a positional constructor to work around this, so an upstream compatibility transition/deprecation phase might be necessary.

Affected code

return LearnAPI.constructor(learner)(NamedTuple{names}(new_values)...)

Possible change

return LearnAPI.constructor(learner)(; NamedTuple{names}(new_values)...) 

PS : this issue have been filled with AI assistance

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions