refactor: ID 型ターゲットのパラメータ化(宣言テーブル化) - #6
Conversation
Collapse the ten copy-pasted ID-datablock target files (60 lines each, differing only in six identifiers) into targets/id_datablock.py: a shared IDDatablockTarget base implementing create_namespace / can_create_from_scope / create_from_scope once, followed by ten attribute-only subclasses that form a declarative table. - Type judgment now lives in one place: can_create_from_scope reads the ol_type/ol_idcode class attributes, and create_from_scope delegates to it (previously triplicated per file). - object.py and collection.py keep their unique scope methods (VIEW3D path, View Layer fallback) verbatim but inherit the shared base and drop their duplicated create_namespace. - material.py remains as a one-line compatibility re-export because tests import the targets.material module path; the other nine files are deleted. - Public class names, registration order, and registry index contents are unchanged (verified by dumping all registry indexes in Blender 5.0.1 before/after: byte-identical). Cluster size: 600 lines -> 189 (178 base+table, 11 shim). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90406f40a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """ | ||
| Mesh リネームターゲット | ||
| """ |
There was a problem hiding this comment.
Keep compatibility shims for deleted target modules
When any existing script, test, or add-on integration imports a per-target module such as from structured_renamer.targets.mesh import MeshRenameTarget, this deletion now raises ModuleNotFoundError; only material.py kept a re-export shim. Since these modules were importable package members before the refactor, keep lightweight shims for the deleted ID target modules or this becomes a breaking change unrelated to the intended behavior-preserving refactor.
Useful? React with 👍 / 👎.
司令塔レビュー(独立検証)— 合格発注書 #3 の受け入れ条件を、請負の報告値を使わず手元で再測定して確認した。
所見
同一 GitHub アカウントのため approve は不可。本コメントをレビュー記録としてマージする。 |
発注書 #3(
dev-note/orders/2026-06-13-id-target-parameterization.md)の成果物。結果サマリー
識別子6個以外完全同一だった ID 型ターゲット10ファイル(material / mesh / armature / action / camera / light / curve / scene / texture / image、各60行 = 600行)を
targets/id_datablock.py1ファイルに畳んだ。IDDatablockTarget約68行 + 宣言テーブル(属性6個だけの class 文10連)約110行、material.py 互換シム 11行)。targets/ 全体で -413行(284 insertions / 697 deletions)ol_type/ol_idcode)のみ。can_create_from_scopeが唯一の判定実装で、create_from_scopeはそこへ委譲。10ファイルにあったTSE_SOME_ID/ID_MA等のリテラル再出現はゼロにIDDatablockTargetを継承してcreate_namespaceを共通化しつつ、固有の収集経路(VIEW3D 選択 / View Layer フォールバック)を持つスコープ判定2メソッドは verbatim に保持新 ID タイプの追加手順(実演 — 実際には追加していない)
例として World を追加する場合:
targets/id_datablock.py末尾に1エントリ追加:targets/__init__.pyの import とtarget_classesに1行ずつ追加メソッド実装はゼロ。発注書の文言「テーブルに1行」に対して実態は「テーブルに1エントリ(属性6行の class 文)」 — これは設計比較の帰結(下記)で、文字どおりの1行(タプル+globals 注入)は静的解析と引き換えになるため見送った。
設計比較と採用理由
実装前に2視点(リロード安全性・レジストリ共生 / 可読性・保守性)の独立判定エージェントで3案を比較した。
MaterialRenameTarget = make_id_target("MaterialRenameTarget", ...))※ B の減点理由は「安全性は満点だが10ファイル分散が目的未達」のみ → 単一モジュール統合(B改)で解消。
決め手: 共通基底がメソッド3つを吸収した時点で、class 文も属性6行だけになりファクトリの「短さ」優位が消える(black の line-length 88 ではファクトリ呼び出しも7〜8行に展開される)。残るのはクラス名文字列の二重化(
__name__ベースのレジストリ重複排除が強制)と、grep "class MaterialRenameTarget"の不発・IDE 定義ジャンプの劣化だけ。素の class 文なら__name__は自然に正しく、リロード時のレジストリ共生(__name__文字列比較での重複排除)も無条件に成立する。案 C は加えて、targets パッケージ本体だけがローダーのリロード対象であるため、毎リロードでクラスが再生成され module dict に幽霊クラスが残存する非対称を生む。
その他の設計判断:
ol_type = OT.TSE_SOME_IDは全 ID 型で共通のため基底に1回だけ宣言create_namespaceはgetattr(data, cls.namespace_key)に一般化(namespace_key と bpy.data の属性名は全12タイプで一致 — 乖離する型が現れたらその時に属性を分離する。YAGNI)targets.materialの import パスをテストが参照するため互換 re-export シムとして残置。他9ファイルは参照ゼロを確認のうえ削除__init_subclass__による属性検証・自動登録は発注書どおり見送り。宣言テーブル(id_datablock.py)と明示リスト(init.py)が並んだ「景色」ができたので、自動登録の採否はこの2つを見比べて判断できる状態発見事項(修正せず記録のみ)
create_from_scopeは OUTLINER 経路で idcode を検査しない(TSE_SOME_IDのみ確認)— 自身のcan_create_from_scopeとは非対称で、ID 型クラスタの「正しい形」より緩い。現状はレジストリ/コレクタがクラス選定済みのため実害はないが、共通実装(can_create 委譲)に乗せると挙動が変わるため、今回は verbatim 保持した。統一するなら別 PR で判断git diff --ignore-cr-at-eolで実質差分を確認可能)pointer_cache引数の型注釈が旧コードではPointerCache(非 Optional)だったが、コレクタは VIEW3D 経路で実際にNoneを渡している。今回触った3ファイルはインターフェース(Optional[PointerCache] = None)に合わせ、None ガードを追加した(当該経路は OUTLINER では到達不能のため挙動同一 — 独立検証でも確認済み)。他のターゲット(bone / node / strip 等)には同じ型の嘘が残っている受け入れ条件チェックリスト
git diff main...HEAD -- tests/は空。Tier 1:uv run pytest= 166 passed + 6 xfailed、Tier 2:tests/blender/run.sh= 39 passed — いずれもベースラインと同一)--factory-startup+BLENDER_USER_SCRIPTS、default_set=Trueで enable → disable、トレースバックなし)dev-note/plans/2026-06-12-milestone.md§3 の該当項目を実績更新🤖 Generated with Claude Code