LearnAPI.clone calls the advertised keyword constructor positionally

Ouverte Adaptée aux débutants
  • Quelque chose ne va pas ?
#67 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

Évaluation

Difficulté
2/5
Temps estimé
1-3 heures
Accessibilité débutants
78/100
Type d'issue
Bug
Clarté
Clairement spécifiée
Activité
Calme
Stack technique
julia
Domaine
api, machine-learning

Piste de recherche

Commencez dans src/clone.jl vers la ligne 26 et comparez l’appel du constructeur avec le contrat du constructeur à mots-clés décrit dans l’issue. Exécutez le reproducteur DemoLearner pour vérifier l’échec, puis confirmez que clone fonctionne avec le constructeur documenté qui n’accepte que des mots-clés et évaluez l’impact sur la compatibilité des constructeurs positionnels existants.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Description

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

Langage dominant
Julia
Étoiles
45
Forks
2
Aucune PR mergée en 30 j

Préparer son environnement

Ce projet ne fournit ni conteneur de développement, ni Dockerfile, ni guide de contribution : l'installation est à votre charge. Commencez par son README, et consultez notre guide de la première contribution pour les étapes générales.

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Autres issues de JuliaAI/LearnAPI.jl

Toutes les issues de JuliaAI/LearnAPI.jl

Issues similaires

Plus d'issues Julia

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.