LearnAPI.clone calls the advertised keyword constructor positionally

Offen Anfängerfreundlich
#67 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

Bewertung

Schwierigkeit
2/5
Geschätzter Aufwand
1-3 Stunden
Anfängerfreundlichkeit
78/100
Issue-Typ
Bug
Klarheit
Klar beschrieben
Aktivitätsstatus
Ruhig
Tech-Stack
julia

Rechercherichtung

Beginnen Sie bei src/clone.jl etwa in Zeile 26 und vergleichen Sie den Konstruktoraufruf mit dem im Issue beschriebenen Vertrag für den Keyword-Konstruktor. Führen Sie den DemoLearner-Reproduzierer aus, um den Fehler zu verifizieren, bestätigen Sie anschließend, dass clone mit dem dokumentierten Keyword-only-Konstruktor funktioniert, und berücksichtigen Sie die Auswirkungen auf die Kompatibilität bestehender Positionskonstruktoren.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Beschreibung

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

https://github.com/JuliaAI/LearnAPI.jl/blob/b17f7dea942464c626e2f7c5267751f7de3705ea/src/clone.jl#L26

Possible change

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

PS : this issue have been filled with AI assistance

Vorherrschende Sprache
Julia
Sterne
45
Forks
2
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Entwicklungsumgebung

Dieses Projekt bietet weder Dev-Container noch Dockerfile noch Beitragsleitfaden – die Einrichtung liegt bei Ihnen. Beginnen Sie mit der README; die allgemeinen Schritte stehen in unserem Leitfaden für den ersten Beitrag.

Erste Schritte

  1. Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
  3. Forken Sie das Repository und arbeiten Sie in einem Branch.
  4. Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.

Mehr aus JuliaAI/LearnAPI.jl

Alle Issues in JuliaAI/LearnAPI.jl

Ähnliche Issues

Weitere Issues zu Julia

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.