Skip to content

feat: add a function to help with prepending arguments to filenames; close #672#4090

Merged
johanneskoester merged 8 commits into
snakemake:mainfrom
edbennett:672-prepend-args
May 5, 2026
Merged

feat: add a function to help with prepending arguments to filenames; close #672#4090
johanneskoester merged 8 commits into
snakemake:mainfrom
edbennett:672-prepend-args

Conversation

@edbennett

@edbennett edbennett commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Per issue #672, there are many situations where you want to pass a list of input files, each prepended by the same command-line option. Currently, this requires a monstrosity of a list comprehension in the shell: directive, or an awkward list comprehension in params:.

This PR provides a helper functionprepend, which makes it easier to generate the argument list in a params: directive.

params:
    data=prepend("--input", input.data)
input:
    data=["a.dat", "b.dat", "c.dat"],
shell:
    "some_tool {params.data}"

will run

some_tool --input a.dat --input b.dat --input c.dat

QC

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • The documentation (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).
  • I, as a human being, have checked each line of code in this pull request

AI-assistance disclosure

I used AI assistance for:

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding

Summary by CodeRabbit

  • New Features

    • Added a utility to prepend flags/prefixes to input filenames or callable-returned inputs, with configurable spacing.
  • Documentation

    • Added a guide for the prepend helper with examples showing spaced and no-space usage and multiple input patterns.
  • Tests

    • Added tests and a sample CLI helper validating behavior for single, multiple, unnamed, and no-space scenarios.

@coderabbitai

coderabbitai Bot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new ioutils helper prepend (Python implementation and export), documents it in the rules docs, and adds tests demonstrating list, single, unnamed, and space=False usages.

Changes

Cohort / File(s) Summary
Core implementation
src/snakemake/ioutils/prepend.py, src/snakemake/ioutils/__init__.py
New prepend(prefix, paths_or_func, space=True) utility that accepts strings, Path, lists, or callables, handles RuleItemProxy wrappers and preserves callable signatures; exported from ioutils.
Documentation
docs/snakefiles/rules.rst
Adds a new section documenting the prepend helper with examples for default (space-separated) and space=False usage (inserted in two locations).
Tests
tests/test_github_issue672/Snakefile, tests/test_github_issue672/test.py, tests/tests.py
New test scenario and CLI helper: Snakefile exercises list/single/unnamed/no-space cases; test.py parses --input; tests/tests.py adds test_github_issue672 to run the scenario.

Sequence Diagram(s)

sequenceDiagram
    participant Rule as "Snakemake Rule"
    participant Prepend as "ioutils.prepend"
    participant Shell as "Shell / Command"
    participant Script as "test.py (CLI)"

    Rule->>Prepend: call prepend(prefix, input(s))
    Prepend-->>Rule: return argument string or callable wrapper
    Rule->>Shell: execute command with constructed args
    Shell->>Script: spawn python test.py with args
    Script-->>Shell: prints parsed inputs
    Shell-->>Rule: command exit / output file created
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.55% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding a prepend helper function to simplify command-line argument construction, with a reference to the related issue #672.
Description check ✅ Passed The description covers the purpose and use case of the new prepend function with clear examples, includes a motivating problem, and all required QC checkboxes are marked as completed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/snakemake/io/container.py (1)

97-103: ⚠️ Potential issue | 🟠 Major

_plainstrings() default now leaks prefixes beyond shell context.

Line 102 and Line 243 default prefixes=True, so callers that don’t opt out now receive prepended CLI fragments instead of raw paths. That violates the “shell-only expansion” behavior for prepend(...).

🔧 Suggested direction
-        prefixes=True,
+        prefixes=False,
...
-    def _plainstrings(self: _TNamedList, prefixes: bool = True) -> _TNamedList:
+    def _plainstrings(self: _TNamedList, prefixes: bool = False) -> _TNamedList:
         return self.__class__.__call__(toclone=self, plainstr=True, prefixes=prefixes)

Also applies to: 243-244

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/io/container.py` around lines 97 - 103, The constructor
parameter prefixes in the class __init__ now defaults to True and similarly the
_plainstrings(...) call/site uses prefixes=True, which causes CLI prefix
fragments to leak outside shell contexts; change the default for prefixes to
False in the class __init__ signature and where _plainstrings is defined/Invoked
(reference the prefixes parameter and the _plainstrings function/method) so that
prepend(...) behavior remains shell-only; update any related docstring/tests
that assume the old default.
🧹 Nitpick comments (1)
docs/snakefiles/rules.rst (1)

806-820: Clarify context behavior and use literal markup consistently.

This section reads as if prefixing is universally applied; add one explicit sentence that prepend affects shell placeholder rendering. Also, use ... markup for inline command/keyword literals for consistent rendering.

📝 Proposed doc patch
 Some tools require adding a flag before each filename they are given.
 To add a flag before each input file in a list,
 the ``prepend`` helper can be used.
+The prefix is applied when formatting into shell commands; in other contexts,
+the underlying filenames remain unchanged.
 For example:
@@
-will run the command `somecommand --input a.txt --input b.txt --input c.txt`.
+will run the command ``somecommand --input a.txt --input b.txt --input c.txt``.
 If spaces are not required between the prefix and the filename,
-set the `space` keyword argument to `False`:
+set the ``space`` keyword argument to ``False``:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/snakefiles/rules.rst` around lines 806 - 820, Clarify that the prepend
helper affects how the shell input placeholder is rendered by adding a sentence
stating that using prepend("--input", [...]) changes the expansion of the
{input} shell placeholder (i.e., the flag is inserted before each filename when
the placeholder is expanded), and update all inline command/keyword mentions to
use literal markup consistently (replace single-backtick instances like
`--input`, `somecommand`, `space` and `{input}` with the documentation's literal
markup ``--input``, ``somecommand``, ``space`` and ``{input}``) so the wording
and examples are unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/snakemake/io/__init__.py`:
- Around line 904-908: The __str__ method of _IOFile has a broken f-string and
accesses self.file (which can raise ValueError for callable-backed IO files);
update _IOFile.__str__ to use self._file for reading the underlying path and fix
the f-string quoting/spacing logic: call is_flagged(self._file, "prepend") and
get_flag_value(self._file, "prepend") and build the returned string by
concatenating prefix + (" " if space_needed else "") + str(self._file) (or
otherwise format without nested/unescaped quotes) so the f-string syntax error
is removed and callable-backed files won't raise on stringification.

In `@src/snakemake/io/fmt.py`:
- Line 23: In fmt_iofile, using str(f.file) bypasses _IOFile.__str__ (losing
prepend(...) behavior); change the call to use str(f) (or otherwise invoke
_IOFile.__str__) so formatted IO strings go through prepend expansion—update the
expression in fmt_iofile that currently assigns f_str = str(f.file) to f_str =
str(f).

In `@tests/tests.py`:
- Around line 3136-3151: The test currently can pass when run(...) does not
raise MissingInputException and also may skip cleanup; update
test_github_issue672 to ensure an exception is expected by either using
pytest.raises(MissingInputException) around the second run(...) call or
explicitly failing if no exception is raised (e.g., call assert False or raise
AssertionError after run(...) when no exception occurs), and move the
shutil.rmtree(tmpdir, ...) cleanup into a finally block so tmpdir is always
removed; reference the test function name test_github_issue672, the run(...)
invocation and the MissingInputException class when making the changes.

---

Outside diff comments:
In `@src/snakemake/io/container.py`:
- Around line 97-103: The constructor parameter prefixes in the class __init__
now defaults to True and similarly the _plainstrings(...) call/site uses
prefixes=True, which causes CLI prefix fragments to leak outside shell contexts;
change the default for prefixes to False in the class __init__ signature and
where _plainstrings is defined/Invoked (reference the prefixes parameter and the
_plainstrings function/method) so that prepend(...) behavior remains shell-only;
update any related docstring/tests that assume the old default.

---

Nitpick comments:
In `@docs/snakefiles/rules.rst`:
- Around line 806-820: Clarify that the prepend helper affects how the shell
input placeholder is rendered by adding a sentence stating that using
prepend("--input", [...]) changes the expansion of the {input} shell placeholder
(i.e., the flag is inserted before each filename when the placeholder is
expanded), and update all inline command/keyword mentions to use literal markup
consistently (replace single-backtick instances like `--input`, `somecommand`,
`space` and `{input}` with the documentation's literal markup ``--input``,
``somecommand``, ``space`` and ``{input}``) so the wording and examples are
unambiguous.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9a3bd2b8-f769-4260-a265-d7370a23c37a

📥 Commits

Reviewing files that changed from the base of the PR and between 29ea226 and 5857cac.

⛔ Files ignored due to path filters (10)
  • tests/test_github_issue672/1.dat is excluded by !**/*.dat
  • tests/test_github_issue672/2.dat is excluded by !**/*.dat
  • tests/test_github_issue672/3.dat is excluded by !**/*.dat
  • tests/test_github_issue672/expected-results/test_all_params.out is excluded by !**/*.out
  • tests/test_github_issue672/expected-results/test_basic.out is excluded by !**/*.out
  • tests/test_github_issue672/expected-results/test_named_params.out is excluded by !**/*.out
  • tests/test_github_issue672/expected-results/test_no_space.out is excluded by !**/*.out
  • tests/test_github_issue672/expected-results/test_single.out is excluded by !**/*.out
  • tests/test_github_issue672/expected-results/test_unnamed_input.out is excluded by !**/*.out
  • tests/test_github_issue672/expected-results/test_wildcards_1.out is excluded by !**/*.out
📒 Files selected for processing (9)
  • docs/snakefiles/rules.rst
  • src/snakemake/io/__init__.py
  • src/snakemake/io/container.py
  • src/snakemake/io/fmt.py
  • src/snakemake/rules.py
  • src/snakemake/workflow.py
  • tests/test_github_issue672/Snakefile
  • tests/test_github_issue672/test.py
  • tests/tests.py

Comment thread src/snakemake/io/__init__.py Outdated
Comment on lines +904 to +908
def __str__(self):
if is_flagged(self.file, "prepend"):
prefix, space_needed = get_flag_value(self.file, "prepend")
return f"{prefix}{" " if space_needed else ""}{self.file}"
return str(self.file)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify parser-level syntax validity for the changed module.
python - <<'PY'
import ast, pathlib, sys
path = pathlib.Path("src/snakemake/io/__init__.py")
src = path.read_text(encoding="utf-8")
try:
    ast.parse(src)
    print("OK: syntax valid")
except SyntaxError as e:
    print(f"SyntaxError: {e}")
    sys.exit(1)
PY

Repository: snakemake/snakemake

Length of output: 121


🏁 Script executed:

#!/bin/bash
# Find the _IOFile class definition and its properties
rg -n "class _IOFile" src/snakemake/io/__init__.py -A 30

Repository: snakemake/snakemake

Length of output: 1320


🏁 Script executed:

#!/bin/bash
# Search for the difference between self.file and self._file in _IOFile
rg -n "self\._?file" src/snakemake/io/__init__.py | head -20

Repository: snakemake/snakemake

Length of output: 1201


🏁 Script executed:

#!/bin/bash
# Find the file property definition in _IOFile
rg -n "def file" src/snakemake/io/__init__.py -A 10 | head -30

Repository: snakemake/snakemake

Length of output: 410


🏁 Script executed:

#!/bin/bash
# Also check the `@property` decorator pattern for file
rg -n "@property" src/snakemake/io/__init__.py -A 5 | grep -A 5 "def file"

Repository: snakemake/snakemake

Length of output: 214


Fix invalid f-string and unsafe property access in _IOFile.__str__.

Line 907 contains a syntax error (nested unescaped double quotes in f-string that prevents module import). Additionally, using self.file raises ValueError for callable-backed IO files; use self._file instead.

Proposed fix
     def __str__(self):
-        if is_flagged(self.file, "prepend"):
-            prefix, space_needed = get_flag_value(self.file, "prepend")
-            return f"{prefix}{" " if space_needed else ""}{self.file}"
-        return str(self.file)
+        value = self._file
+        if is_flagged(value, "prepend"):
+            prefix, space_needed = get_flag_value(value, "prepend")
+            return f"{prefix}{' ' if space_needed else ''}{value}"
+        return str(value)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/io/__init__.py` around lines 904 - 908, The __str__ method of
_IOFile has a broken f-string and accesses self.file (which can raise ValueError
for callable-backed IO files); update _IOFile.__str__ to use self._file for
reading the underlying path and fix the f-string quoting/spacing logic: call
is_flagged(self._file, "prepend") and get_flag_value(self._file, "prepend") and
build the returned string by concatenating prefix + (" " if space_needed else
"") + str(self._file) (or otherwise format without nested/unescaped quotes) so
the f-string syntax error is removed and callable-backed files won't raise on
stringification.

Comment thread src/snakemake/io/fmt.py Outdated
f_str = f.storage_object.print_query
else:
f_str = f
f_str = str(f.file)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

fmt_iofile now bypasses prepend expansion for non-storage files.

Line 23 uses str(f.file), which skips _IOFile.__str__ and loses prepend(...) behavior in formatted IO strings (including shell-facing formatting paths).

🐛 Proposed fix
-        f_str = str(f.file)
+        f_str = str(f)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
f_str = str(f.file)
f_str = str(f)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/io/fmt.py` at line 23, In fmt_iofile, using str(f.file)
bypasses _IOFile.__str__ (losing prepend(...) behavior); change the call to use
str(f) (or otherwise invoke _IOFile.__str__) so formatted IO strings go through
prepend expansion—update the expression in fmt_iofile that currently assigns
f_str = str(f.file) to f_str = str(f).

Comment thread tests/tests.py Outdated
Comment on lines +3136 to +3151
def test_github_issue672():
path = dpath("test_github_issue672")

# Ensure that arguments are correctly passed to Python
tmpdir = run(path, cleanup=False)

# Ensure that filenames are correctly formatted
# when Snakemake reports them for other reasons
(tmpdir / "3.dat").unlink()
(tmpdir / "test_basic.out").unlink()
try:
run(path, tmpdir=tmpdir, cleanup=False)
except MissingInputException as ex:
assert ex.args[0].split("\n")[-1].strip() == "3.dat"

shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Test can pass even when the expected exception is never raised.

Current control flow only asserts inside except, so this test can silently pass if run(...) succeeds. Also, cleanup should be guaranteed via finally.

✅ Proposed fix
 def test_github_issue672():
     path = dpath("test_github_issue672")
@@
     (tmpdir / "3.dat").unlink()
     (tmpdir / "test_basic.out").unlink()
-    try:
-        run(path, tmpdir=tmpdir, cleanup=False)
-    except MissingInputException as ex:
-        assert ex.args[0].split("\n")[-1].strip() == "3.dat"
-
-    shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)
+    try:
+        try:
+            run(path, tmpdir=tmpdir, cleanup=False)
+        except MissingInputException as ex:
+            assert ex.args[0].split("\n")[-1].strip() == "3.dat"
+        else:
+            raise AssertionError("Expected MissingInputException was not raised.")
+    finally:
+        shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_github_issue672():
path = dpath("test_github_issue672")
# Ensure that arguments are correctly passed to Python
tmpdir = run(path, cleanup=False)
# Ensure that filenames are correctly formatted
# when Snakemake reports them for other reasons
(tmpdir / "3.dat").unlink()
(tmpdir / "test_basic.out").unlink()
try:
run(path, tmpdir=tmpdir, cleanup=False)
except MissingInputException as ex:
assert ex.args[0].split("\n")[-1].strip() == "3.dat"
shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)
def test_github_issue672():
path = dpath("test_github_issue672")
# Ensure that arguments are correctly passed to Python
tmpdir = run(path, cleanup=False)
# Ensure that filenames are correctly formatted
# when Snakemake reports them for other reasons
(tmpdir / "3.dat").unlink()
(tmpdir / "test_basic.out").unlink()
try:
try:
run(path, tmpdir=tmpdir, cleanup=False)
except MissingInputException as ex:
assert ex.args[0].split("\n")[-1].strip() == "3.dat"
else:
raise AssertionError("Expected MissingInputException was not raised.")
finally:
shutil.rmtree(tmpdir, ignore_errors=ON_WINDOWS)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/tests.py` around lines 3136 - 3151, The test currently can pass when
run(...) does not raise MissingInputException and also may skip cleanup; update
test_github_issue672 to ensure an exception is expected by either using
pytest.raises(MissingInputException) around the second run(...) call or
explicitly failing if no exception is raised (e.g., call assert False or raise
AssertionError after run(...) when no exception occurs), and move the
shutil.rmtree(tmpdir, ...) cleanup into a finally block so tmpdir is always
removed; reference the test function name test_github_issue672, the run(...)
invocation and the MissingInputException class when making the changes.

@johanneskoester

johanneskoester commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

TBH, I think I would rather not want parameter syntax like this to live in the input directive. Imagine rule inheritance for example. Anybody replacing the input during that with different paths will have to do this again. However, I definitely see the utility of this and would thus suggest the following syntax instead, in line with other helper functions that we already have (https://snakemake.readthedocs.io/en/stable/snakefiles/rules.html#semantic-helpers):

rule a:
    input:
        data=["a.dat", "b.dat", "c.dat"]
    params:
        prepend("--input ", input.data)  # note that we here make use of the rule item access helpers, obviating the need for defining a new function here (https://snakemake.readthedocs.io/en/stable/snakefiles/rules.html#rule-item-access-helpers)
    shell:
        "some_tool {input}"

@edbennett

Copy link
Copy Markdown
Contributor Author

Makes sense, thanks!

…lose snakemake#672

Per issue snakemake#672, there are many situations where you want to pass a list of input files, each prepended by the same command-line option. Currently, this requires a monstrosity of a list comprehension in the `shell:` directive, or an awkward list comprehension in `params:`.

This PR provides a helper function`prepend`, which makes it easier to generate the argument list in a `params:` directive.

    params:
        data=prepend("--input", input.data)
    input:
        data=["a.dat", "b.dat", "c.dat"],
    shell:
        "some_tool {params.data}"

will run

    some_tool --input a.dat --input b.dat --input c.dat
@edbennett edbennett changed the title feat: add a function indicating that filenames should have an argument prepended; close #672 feat: add a function to help with prepending arguments to filenames; close #672 Mar 13, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/snakemake/ioutils/prepend.py`:
- Around line 27-31: The ValueError in prepend.py currently references the wrong
helper name "subpath" which is misleading; update the exception message inside
the validation that raises ValueError to mention "prepend" (or the actual helper
name used) instead of "subpath" so errors accurately identify the helper, e.g.
adjust the message built around repr(path) in the block that raises ValueError
in the prepend helper/function.
- Around line 35-36: The code path that checks "if isinstance(paths,
RuleItemProxy)" always resolves the proxy as "input" by returning "lambda
wildcards, input: do(input)" which ignores RuleItemProxy.name; update this
branch to inspect the RuleItemProxy.name and return a lambda that passes the
correct rule item (e.g., input, output, params, resources) into do(...) instead
of hardcoding "input", using the proxy.name value to select the proper argument
name in the returned lambda so RuleItemProxy resolution is correct for non-input
proxies.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: da7c9ed3-4b1f-4619-b37e-fe215e30f3fc

📥 Commits

Reviewing files that changed from the base of the PR and between 03d483a and bfcc06b.

⛔ Files ignored due to path filters (7)
  • tests/test_github_issue672/1.dat is excluded by !**/*.dat
  • tests/test_github_issue672/2.dat is excluded by !**/*.dat
  • tests/test_github_issue672/3.dat is excluded by !**/*.dat
  • tests/test_github_issue672/expected-results/test_basic.out is excluded by !**/*.out
  • tests/test_github_issue672/expected-results/test_no_space.out is excluded by !**/*.out
  • tests/test_github_issue672/expected-results/test_single.out is excluded by !**/*.out
  • tests/test_github_issue672/expected-results/test_unnamed_input.out is excluded by !**/*.out
📒 Files selected for processing (6)
  • docs/snakefiles/rules.rst
  • src/snakemake/ioutils/__init__.py
  • src/snakemake/ioutils/prepend.py
  • tests/test_github_issue672/Snakefile
  • tests/test_github_issue672/test.py
  • tests/tests.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/tests.py
  • tests/test_github_issue672/Snakefile

Comment thread src/snakemake/ioutils/prepend_param.py
Comment thread src/snakemake/ioutils/prepend.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
src/snakemake/ioutils/prepend.py (2)

27-31: ⚠️ Potential issue | 🟡 Minor

Fix misleading helper name in the validation error.

Line 29 references subpath, but this helper is prepend. This makes error diagnostics confusing.

Suggested fix
-                "Values passed to subpath "
+                "Values passed to prepend "
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/ioutils/prepend.py` around lines 27 - 31, The ValueError
message in the prepend helper wrongly mentions "subpath"; update the validation
error in the prepend function to reference "prepend" (or "prepend helper") so
diagnostics are accurate: locate the check in prepend that raises ValueError for
non-str/non-pathlib inputs (the block referencing variable path) and change the
error string to say "Values passed to prepend must be..." while preserving the
repr(path) detail and existing wording.

35-37: ⚠️ Potential issue | 🟠 Major

Resolve RuleItemProxy via paths.name instead of hardcoded input.

Line 37 hardcodes input, so non-input proxies can resolve incorrectly. It also shadows Python’s builtin input.

Suggested fix
     def do(paths):
         if isinstance(paths, RuleItemProxy):
-            return lambda wildcards, input: do(input)
+            item_name = paths.name
+
+            def inner(wildcards, **kwargs):
+                return do(kwargs[item_name])
+
+            overwrite_function_params(inner, ["wildcards", item_name])
+            return inner
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/snakemake/ioutils/prepend.py` around lines 35 - 37, The lambda returned
from do currently hardcodes the parameter name "input" and shadows Python's
builtin; update do so that when paths is a RuleItemProxy you return a lambda
using a kwargs capture and lookup by the proxy's name: change the lambda from
"lambda wildcards, input: do(input)" to "lambda wildcards, **kwargs:
do(kwargs[paths.name])" (i.e., in the do function, detect RuleItemProxy and
resolve via paths.name from kwargs instead of the hardcoded input identifier).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/snakemake/ioutils/prepend.py`:
- Around line 27-31: The ValueError message in the prepend helper wrongly
mentions "subpath"; update the validation error in the prepend function to
reference "prepend" (or "prepend helper") so diagnostics are accurate: locate
the check in prepend that raises ValueError for non-str/non-pathlib inputs (the
block referencing variable path) and change the error string to say "Values
passed to prepend must be..." while preserving the repr(path) detail and
existing wording.
- Around line 35-37: The lambda returned from do currently hardcodes the
parameter name "input" and shadows Python's builtin; update do so that when
paths is a RuleItemProxy you return a lambda using a kwargs capture and lookup
by the proxy's name: change the lambda from "lambda wildcards, input: do(input)"
to "lambda wildcards, **kwargs: do(kwargs[paths.name])" (i.e., in the do
function, detect RuleItemProxy and resolve via paths.name from kwargs instead of
the hardcoded input identifier).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 056110f7-b67d-4555-ac66-7e48a72de416

📥 Commits

Reviewing files that changed from the base of the PR and between bfcc06b and 9495403.

📒 Files selected for processing (1)
  • src/snakemake/ioutils/prepend.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/snakemake/ioutils/prepend.py`:
- Around line 16-22: Update the docstring in the prepend function to use the
correct parameter name: change the mention of `path_or_func` to `paths_or_func`
so it matches the actual parameter `paths_or_func` in this module (look for the
prepend function in src/snakemake/ioutils/prepend.py and update its
triple-quoted description accordingly).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1265024f-ad53-464a-8f0a-0c2880a3894b

📥 Commits

Reviewing files that changed from the base of the PR and between 9495403 and 5b98f6c.

📒 Files selected for processing (1)
  • src/snakemake/ioutils/prepend.py

Comment thread src/snakemake/ioutils/prepend_param.py

@johanneskoester johanneskoester left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Just two minor things.

Comment thread docs/snakefiles/rules.rst Outdated
Comment thread docs/snakefiles/rules.rst Outdated
@edbennett

Copy link
Copy Markdown
Contributor Author

Done!

@johanneskoester johanneskoester merged commit 14ccd1d into snakemake:main May 5, 2026
59 checks passed
johanneskoester pushed a commit that referenced this pull request May 14, 2026
🤖 I have created a release *beep* *boop*
---


##
[9.21.0](v9.20.0...v9.21.0)
(2026-05-13)


### Features

* add a function to help with prepending arguments to filenames; close
[#672](#672)
([#4090](#4090))
([14ccd1d](14ccd1d))


### Bug Fixes

* close plugin handlers after draining QueueListener in
LoggerManager.stop()
([#4137](#4137))
([b2a9e69](b2a9e69))


### Performance Improvements

* adjust default sqlite PRAGMAs, auto detect network fstype
([#4152](#4152))
([3df2d35](3df2d35))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Inline join/expand syntax to support args into multiple command line arguments

2 participants