Repository navigation
fix: defer params functions touching named input of incomplete expansion - #4331
barlowa124 wants to merge 1 commit into
Conversation
An input function that resolves a checkpoint and returns a dict of named inputs crashes the workflow when a params function accesses such a name. Params are evaluated while the job input still holds only the checkpoint placeholder, so the names do not exist yet. Treat a missing-name AttributeError on the injected named lists the same as a FileNotFoundError on injected input files when the job's input expansion is incomplete. The parameter becomes <TBD> and is re-evaluated once the checkpoint has run and the job is updated.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to Checkpoint-dependent params are reevaluated after the checkpoint provides the named inputs. No actionable merge risk remains beyond normal checks. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (2 skipped: 2 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Linked Issues check Issue #4312requires named inputs returned from a checkpoint input function and exposed withunpack()to be accessible in a params function.Rule.apply_input_functionnow defers a missing-name `At…Out of Scope Changes check The reported changes are limited to deferring parameter evaluation for incomplete checkpoint input expansion and adding its regression test and expected output. These changes directly support issue #4…Title check The title clearly and concisely describes the main change: deferring parameter functions that access named inputs during incomplete expansion. Description check The description explains the bug, implementation approach, expected behavior, test coverage, documentation decision, human review, and AI assistance. It follows the repository template.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (2 skipped: 2 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Closes #4312.
When a checkpoint-resolving input function returns named inputs via
unpack(), a params function that reads one of those names fails.Parameter evaluation happens during DAG construction (in this case via
collect_envs->software_env_spec), while the job input stillcarries only the checkpoint placeholder. The injected
InputFilesdoes not yet have the names, so
AttributeErrorpropagates.This mirrors the
FileNotFoundErrordeferral already used forinput-dependent functions: while the job's input expansion is
incomplete, a missing-name
AttributeErroron the injected listsresolves to
<TBD>. After the checkpoint runs and the job isreplaced, the parameter resolves normally on re-evaluation.
Coverage:
tests/test_github_issue4312reproduces the reportedAttributeErroron unpatched code.QC
docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).AI-assistance disclosure
I used AI assistance for the categories checked below.