LearnAPI.clone calls the advertised keyword constructor positionally
まだ誰も着手していません。
評価
- 難易度
- 2/5
- 見積もり時間
- 1〜3時間
- 初心者へのやさしさ
- 78/100
- issue の種類
- バグ
- 明瞭さ
- 明確に書かれている
- 活発さ
- 静か
- 技術スタック
- julia
- 領域
- api, machine-learning
調査の方向性
src/clone.jl の 26 行目付近から始め、コンストラクター呼び出しを issue に記載されたキーワードコンストラクターの契約と比較してください。DemoLearner の再現コードを実行して失敗を確認し、その後、clone がドキュメントに記載されたキーワード専用コンストラクターで動作することを確認して、既存の位置引数コンストラクターへの互換性の影響を検討してください。
索引モデルが issue の本文から書いたものです。
説明
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
Possible change
return LearnAPI.constructor(learner)(; NamedTuple{names}(new_values)...)
PS : this issue have been filled with AI assistance
- 主要言語
- Julia
- スター
- 45
- フォーク
- 2
- PR マージ指標
- 30日以内にマージされた PR はありません
環境構築
このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
JuliaAI/LearnAPI.jl のほかの issue
-
難易度 5/5 1週間以上 初心者へのやさしさ 25/100
JuliaAI/LearnAPI.jl#19 ·
JuliaAI/LearnAPI.jl の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
CliMA/ClimaArtifacts#184 ·
-
In-place Vern7 stiffness estimate uses mismatched stage values対応中かも @devmotion が今日担当しました。 オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
SciML/OrdinaryDiffEq.jl#4778 ·
メンテナーはふだん 1 日以内に返信
-
`pick_batchsize` spends ~1 µs constructing `BatchSizeSettings{B}(N)` with a run-time `B`対応中かも @devmotion が今日担当しました。 オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 68/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 86/100
JuliaDiff/ReverseDiff.jl#318 ·
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100