Skip to content

Conversation

@tdayris
Copy link
Contributor

@tdayris tdayris commented Dec 5, 2025

This PR brings go-yq to the list of available wrappers. I called this wrapper go-yq and not simply yq since it refers to the conda-forge/go-yq package which is not the same as conda-forge/yq. Yet, both have the same name though CLI, and a shared history.

QC

While the contributions guidelines are more extensive, please particularly ensure that:

  • test.py was updated to call any added or updated example rules in a Snakefile
  • input: and output: file paths in the rules can be chosen arbitrarily
  • wherever possible, command line arguments are inferred and set automatically (e.g. based on file extensions in input: or output:)
  • temporary files are either written to a unique hidden folder in the working directory, or (better) stored where the Python function tempfile.gettempdir() points to
  • the meta.yaml contains a link to the documentation of the respective tool or command under url:
  • conda environments use a minimal amount of channels and packages, in recommended ordering

Summary by CodeRabbit

  • New Features

    • Added a go-yq wrapper for YAML/CSV processing: concatenation, format conversion, content updates, expression evaluation, splitting, and automatic format detection/validation.
  • Tests

    • Added end-to-end tests exercising concat, update, evaluate, split, and format-conversion scenarios.
  • Chores

    • Added reproducible environment specifications and package metadata for the new wrapper.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 5, 2025

📝 Walkthrough

Walkthrough

Adds a new go-yq Snakemake wrapper: implementation, metadata, conda env specs (including a pinned linux-64 file), test fixtures and Snakefile, and a test function in test_wrappers.py to run the wrapper integration tests.

Changes

Cohort / File(s) Summary
Wrapper implementation
utils/go-yq/wrapper.py
New Python wrapper implementing detect_fmt(files, long_option, short_option, extra), inferring input/output formats from filenames, assembling extra flags, quoting expressions, validating multi-output split-exp and disallowing inplace ops, and invoking yq with constructed args and logging.
Conda environment
utils/go-yq/environment.yaml, utils/go-yq/environment.linux-64.pin.txt
New environment spec declaring go-yq and snakemake-wrapper-utils and an explicit linux-64 pin file enumerating exact package artifacts for reproducible environment creation.
Wrapper metadata
utils/go-yq/meta.yaml
New metadata describing the wrapper (name, URL, description, authors), inputs/outputs, params (extra, expression, subcommand), and usage notes about threading, format detection, and splitting.
Tests / Snakefile
utils/go-yq/test/Snakefile
New test rules exercising concat, YAML update, evaluate, split (requires -s/--split-exp), and format conversion (CSV→JSON) via the wrapper.
Test fixtures
utils/go-yq/test/bazquux.yaml, utils/go-yq/test/foobar.yaml, utils/go-yq/test/eval.yaml, utils/go-yq/test/table.csv
New test input files: simple key/value, multi-document YAML, evaluation data with a path expression, and a CSV for format-conversion tests.
Test harness
test_wrappers.py
Adds test_go_yq(run) to invoke the wrapper tests via Snakemake with --cores 1 --use-conda -F.

Sequence Diagram(s)

sequenceDiagram
    participant Snakemake
    participant CondaEnv as Conda Env
    participant Wrapper as utils/go-yq/wrapper.py
    participant FS as Filesystem
    participant YQ as go-yq (yq)

    Snakemake->>CondaEnv: provision environment (environment.yaml / pin)
    Snakemake->>Wrapper: invoke wrapper with inputs, outputs, params
    Wrapper->>FS: inspect filenames (detect_fmt)
    Wrapper->>Wrapper: validate formats, split-exp, inplace rules
    Wrapper->>YQ: exec `yq {subcommand} {extra} {expr} {infile} {outfile}`
    YQ-->>FS: read inputs / write outputs
    YQ-->>Wrapper: return exit status & logs
    Wrapper-->>Snakemake: return status and log path
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Focus review on utils/go-yq/wrapper.py (format detection logic, flag assembly, quoting/escaping of expressions, multi-output split handling).
  • Verify utils/go-yq/test/Snakefile test expectations match wrapper behavior (especially split-exp and format inference).
  • Quick scan of environment.linux-64.pin.txt for consistency with environment.yaml.

Suggested reviewers

  • fgvieira
  • johanneskoester
  • dlaehnemann

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: go-yq' uses conventional commit style and accurately describes the main change—adding a new go-yq wrapper to the repository.
Description check ✅ Passed The PR description follows the template, explains the distinction between go-yq and yq packages, and all QC checklist items are marked as completed with specific confirmations.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 81915d7 and 3acbf49.

📒 Files selected for processing (2)
  • test_wrappers.py (1 hunks)
  • utils/go-yq/test/Snakefile (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • utils/go-yq/test/Snakefile
  • test_wrappers.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: docs
  • GitHub Check: testing
  • GitHub Check: Summary

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (2)
utils/go-yq/meta.yaml (1)

17-25: Consider minor wording polish in notes

The semantics are clear, but a few small grammar tweaks would improve readability:

-  Format are mostly auto-detected using `--output-format` and `--input-format`.
+  Formats are mostly auto-detected using `--output-format` and `--input-format`.

-  Through file splitting, files name expression are not automatically inferred.
-  User should provide the proper `-s/--split-exp` parameters in `params.extra`.
+  For file splitting, file name expressions are not automatically inferred.
+  Users should provide the appropriate `-s/--split-exp` parameters in `params.extra`.

Purely cosmetic; feel free to apply now or later.

test_wrappers.py (1)

7161-7176: Extend test_go_yq targets to exercise split-output rule

Right now the test only requests concat.yaml, updated.yaml, evaluated.yaml, and table.json, so the test_yq_split_yaml rule (and the wrapper’s multi-output -s/--split-exp logic) is never run.

To cover that code path as well, you can include the split outputs:

         [
             "snakemake",
             "--cores",
             "1",
             "--use-conda",
             "-F",
             "concat.yaml",
             "updated.yaml",
             "evaluated.yaml",
+            "foo_bar.yaml",
+            "foo_bar2.yaml",
             "table.json",
         ],

This keeps the test pattern consistent while adding coverage for the split behavior.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d0d7034 and 9431b05.

⛔ Files ignored due to path filters (1)
  • utils/go-yq/test/table.csv is excluded by !**/*.csv
📒 Files selected for processing (9)
  • test_wrappers.py (1 hunks)
  • utils/go-yq/environment.linux-64.pin.txt (1 hunks)
  • utils/go-yq/environment.yaml (1 hunks)
  • utils/go-yq/meta.yaml (1 hunks)
  • utils/go-yq/test/Snakefile (1 hunks)
  • utils/go-yq/test/bazquux.yaml (1 hunks)
  • utils/go-yq/test/eval.yaml (1 hunks)
  • utils/go-yq/test/foobar.yaml (1 hunks)
  • utils/go-yq/wrapper.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py

⚙️ CodeRabbit configuration file

**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.

Files:

  • utils/go-yq/wrapper.py
  • test_wrappers.py
**/wrapper.py

⚙️ CodeRabbit configuration file

Do not complain about use of undefined variable called snakemake.

Files:

  • utils/go-yq/wrapper.py
🧠 Learnings (21)
📓 Common learnings
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/wrapper.py:27-28
Timestamp: 2024-11-26T08:30:23.818Z
Learning: In Snakemake wrappers (e.g., `wrapper.py` files), it's unnecessary to verify the availability of tools like `mtnucratio` within the code, because Snakemake with Conda ensures that the required tools are installed and available.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3502
File: bio/ngsbits/sampleancestry/wrapper.py:18-23
Timestamp: 2024-11-26T14:59:03.678Z
Learning: In Snakemake wrapper scripts, Snakemake validates input and output paths, so explicit shell quoting is not necessary.
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/preprocess-variants/wrapper.py:0-0
Timestamp: 2024-11-15T13:48:33.759Z
Learning: In Snakemake wrappers, security considerations like input sanitization are unnecessary, as the wrappers are under full control of the user.
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-21T10:23:03.427Z
Learning: In the Snakemake wrappers project, avoid suggesting extensive error handling or temporary file management in simple wrapper scripts when it may be unnecessary, to prevent overcomplicating the code.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/test/Snakefile:58-71
Timestamp: 2024-11-26T10:49:54.765Z
Learning: In test Snakefiles within the snakemake-wrappers repository, it is acceptable to use simplified paths and logging configurations that may differ from real-life pipelines.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22
Timestamp: 2024-11-26T09:16:39.570Z
Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.
Learnt from: tedil
Repo: snakemake/snakemake-wrappers PR: 4274
File: bio/mehari/annotate-seqvars/wrapper.py:11-19
Timestamp: 2025-06-15T07:43:03.263Z
Learning: In Snakemake wrappers, a common pattern is to have flag variables that are either empty strings "" or the actual flag strings (e.g., "--keep-intergenic") for direct interpolation in shell commands. This avoids conditionals in the shell expression and keeps the command construction clean.
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3123
File: utils/datavzrd/wrapper.py:31-32
Timestamp: 2024-08-21T08:30:42.757Z
Learning: In `wrapper.py` scripts, do not flag the use of an undefined variable called `snakemake`.
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3123
File: utils/datavzrd/wrapper.py:31-32
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In `wrapper.py` scripts, do not flag the use of an undefined variable called `snakemake`.
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3123
File: utils/datavzrd/wrapper.py:31-32
Timestamp: 2024-10-08T17:41:54.542Z
Learning: The `snakemake` variable is inserted via a preamble during execution in `wrapper.py` scripts, so it doesn't need to be explicitly defined.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 4285
File: bio/agat/wrapper.py:336-340
Timestamp: 2025-06-20T13:10:29.240Z
Learning: User tdayris prefers to fix identified code issues immediately rather than deferring them, as indicated by their comment "Now is better than later" when confirming code fixes.
📚 Learning: 2025-04-17T09:24:51.738Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 0
File: :0-0
Timestamp: 2025-04-17T09:24:51.738Z
Learning: In snakemake-wrappers repository, environment.yaml files should follow these conventions:
1. Use whitespace before the equal sign in version specifications (e.g., "datavzrd =2.53.1")
2. Only specify the exact version for the main software package
3. Don't add version constraints for dependencies unless absolutely necessary
4. See full guidelines at: https://snakemake-wrappers.readthedocs.io/en/stable/contributing.html#environment-yaml-file

Applied to files:

  • utils/go-yq/environment.yaml
  • utils/go-yq/environment.linux-64.pin.txt
📚 Learning: 2025-04-17T09:24:51.738Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 0
File: :0-0
Timestamp: 2025-04-17T09:24:51.738Z
Learning: In snakemake-wrappers repository, environment.yaml files should follow these conventions:
1. Use whitespace before the equal sign in version specifications (e.g., "datavzrd =2.53.1")
2. Only specify the exact version for the main software package
3. Don't add version constraints for dependencies unless absolutely necessary
4. See guidelines at: https://snakemake-wrappers.readthedocs.io/en/stable/contributing.html#environment-yaml-file

Applied to files:

  • utils/go-yq/environment.yaml
  • utils/go-yq/environment.linux-64.pin.txt
📚 Learning: 2025-02-11T12:24:22.592Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 3648
File: bio/nanosim/simulator/environment.yaml:6-6
Timestamp: 2025-02-11T12:24:22.592Z
Learning: In the nanosim bioconda recipe, dependencies are carefully managed with specific version pins (e.g., scikit-learn ~=0.23.2) to ensure compatibility with pre-trained models. These dependencies don't need to be explicitly added to environment.yaml files when the main package is listed as a dependency, as they are handled through the bioconda recipe system.

Applied to files:

  • utils/go-yq/environment.yaml
  • utils/go-yq/environment.linux-64.pin.txt
📚 Learning: 2025-06-02T07:56:35.854Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 4159
File: bio/pyfaidx/environment.yaml:6-6
Timestamp: 2025-06-02T07:56:35.854Z
Learning: In the Snakemake-wrapper repository, conda dependency version pinning in environment.yaml files uses spaces around the equals sign (e.g., `- pyfaidx =0.8.1.4`) as the established coding standard, even though conda itself doesn't require the spaces.

Applied to files:

  • utils/go-yq/environment.yaml
  • utils/go-yq/environment.linux-64.pin.txt
📚 Learning: 2024-11-26T09:16:39.570Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22
Timestamp: 2024-11-26T09:16:39.570Z
Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.

Applied to files:

  • utils/go-yq/test/Snakefile
📚 Learning: 2024-11-26T08:31:00.099Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/test/Snakefile:2-6
Timestamp: 2024-11-26T08:31:00.099Z
Learning: In test files for Snakemake wrappers, such as `bio/mtnucratio/test/Snakefile`, hard-coded input and output paths are acceptable as examples and do not need to use wildcards to make paths flexible.

Applied to files:

  • utils/go-yq/test/Snakefile
📚 Learning: 2024-11-26T09:16:24.981Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:1-13
Timestamp: 2024-11-26T09:16:24.981Z
Learning: In test `Snakefile`s (e.g., `test/Snakefile`), it's acceptable to use fixed input and output file names instead of wildcards.

Applied to files:

  • utils/go-yq/test/Snakefile
📚 Learning: 2024-11-26T10:49:54.765Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/test/Snakefile:58-71
Timestamp: 2024-11-26T10:49:54.765Z
Learning: In test Snakefiles within the snakemake-wrappers repository, it is acceptable to use simplified paths and logging configurations that may differ from real-life pipelines.

Applied to files:

  • utils/go-yq/test/Snakefile
📚 Learning: 2024-11-26T14:59:03.678Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3502
File: bio/ngsbits/sampleancestry/wrapper.py:18-23
Timestamp: 2024-11-26T14:59:03.678Z
Learning: In Snakemake wrapper scripts, Snakemake validates input and output paths, so explicit shell quoting is not necessary.

Applied to files:

  • utils/go-yq/test/Snakefile
  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-26T15:01:13.202Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3502
File: bio/ngsbits/sampleancestry/wrapper.py:1-23
Timestamp: 2024-11-26T15:01:13.202Z
Learning: The NGS-bits SampleAncestry wrapper in `bio/ngsbits/sampleancestry/` includes a test Snakefile, sample VCF files, and tests available in the `test/` directory.

Applied to files:

  • utils/go-yq/test/Snakefile
📚 Learning: 2024-11-21T10:23:03.427Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-21T10:23:03.427Z
Learning: In the Snakemake wrappers project, avoid suggesting extensive error handling or temporary file management in simple wrapper scripts when it may be unnecessary, to prevent overcomplicating the code.

Applied to files:

  • utils/go-yq/test/Snakefile
  • utils/go-yq/wrapper.py
📚 Learning: 2024-08-21T08:30:42.757Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3123
File: utils/datavzrd/wrapper.py:31-32
Timestamp: 2024-08-21T08:30:42.757Z
Learning: In `wrapper.py` scripts, do not flag the use of an undefined variable called `snakemake`.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-26T08:30:23.818Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/wrapper.py:27-28
Timestamp: 2024-11-26T08:30:23.818Z
Learning: In Snakemake wrappers (e.g., `wrapper.py` files), it's unnecessary to verify the availability of tools like `mtnucratio` within the code, because Snakemake with Conda ensures that the required tools are installed and available.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2025-06-15T07:43:03.263Z
Learnt from: tedil
Repo: snakemake/snakemake-wrappers PR: 4274
File: bio/mehari/annotate-seqvars/wrapper.py:11-19
Timestamp: 2025-06-15T07:43:03.263Z
Learning: In Snakemake wrappers, a common pattern is to have flag variables that are either empty strings "" or the actual flag strings (e.g., "--keep-intergenic") for direct interpolation in shell commands. This avoids conditionals in the shell expression and keeps the command construction clean.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-15T13:44:18.810Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-15T13:44:18.810Z
Learning: In the snakemake-wrappers repository, it is acceptable to use f-strings with `shell=True` in wrapper scripts (e.g., `wrapper.py` files), as these wrappers are under full control of the user. Potential command injection vulnerabilities are not considered an issue in this context.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2025-06-04T06:32:20.090Z
Learnt from: rohan-ibn-tariq
Repo: snakemake/snakemake-wrappers PR: 4160
File: bio/trf/wrapper.py:18-26
Timestamp: 2025-06-04T06:32:20.090Z
Learning: For Snakemake wrappers, it's preferable to keep parameter dictionaries and constants directly visible in wrapper.py files rather than importing from config modules, to maintain minimal, self-documenting code that doc viewers can understand at a glance.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-15T13:48:33.759Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/preprocess-variants/wrapper.py:0-0
Timestamp: 2024-11-15T13:48:33.759Z
Learning: In Snakemake wrappers, security considerations like input sanitization are unnecessary, as the wrappers are under full control of the user.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-15T18:36:04.660Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/call-variants/wrapper.py:15-23
Timestamp: 2024-11-15T18:36:04.660Z
Learning: In the Snakemake wrappers repository, using `shell=True` and redirecting within shell commands is acceptable.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-09-04T18:46:15.788Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 3165
File: bio/nanosim/simulator/wrapper.py:1-163
Timestamp: 2024-09-04T18:46:15.788Z
Learning: In the `wrapper.py` script for the NanoSim Snakemake wrapper, the subcommand is inferred from the pattern of named input files, rather than being explicitly specified.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2025-01-30T14:19:53.384Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 3432
File: bio/reference/ensembl-sequence/wrapper.py:79-87
Timestamp: 2025-01-30T14:19:53.384Z
Learning: In Snakemake wrappers, error reporting should use `print(..., file=sys.stderr)` for direct error messages, while `snakemake.log_fmt_shell()` is used for capturing command output and errors.

Applied to files:

  • utils/go-yq/wrapper.py
🪛 Ruff (0.14.7)
utils/go-yq/wrapper.py

34-37: Avoid specifying long messages outside the exception class

(TRY003)


50-50: Undefined name snakemake

(F821)


51-51: Undefined name snakemake

(F821)


52-52: Undefined name snakemake

(F821)


54-54: Undefined name snakemake

(F821)


56-56: Undefined name snakemake

(F821)


59-59: Undefined name snakemake

(F821)


64-64: Undefined name snakemake

(F821)


65-65: Undefined name snakemake

(F821)


67-70: Avoid specifying long messages outside the exception class

(TRY003)


71-71: Undefined name snakemake

(F821)


74-74: Undefined name snakemake

(F821)


75-75: Undefined name snakemake

(F821)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: docs
  • GitHub Check: testing
  • GitHub Check: Summary
🔇 Additional comments (7)
utils/go-yq/test/foobar.yaml (1)

1-4: YAML test fixture looks good

Two simple documents with the expected keys; suitable as input for the concat/update tests.

utils/go-yq/environment.yaml (1)

1-5: Environment specification aligns with wrapper conventions

Single pinned dependency (go-yq =4.49.2) on conda-forge and spacing around = follow the documented environment.yaml guidelines; no changes needed.

Based on learnings, this matches the expected env.yaml style.

utils/go-yq/test/eval.yaml (1)

1-5: Eval test YAML matches intended structure

The pathExp scalar and nested .a.b[] list are well-formed and consistent with the eval(.pathExp) usage in the Snakefile.

utils/go-yq/test/bazquux.yaml (1)

1-1: Minimal concat input fixture is fine

Single-key YAML mapping is valid and appropriate for exercising the concat rule.

utils/go-yq/environment.linux-64.pin.txt (1)

1-10: Explicit pin file looks appropriately generated

The @explicit linux-64 pin list (toolchain libs plus go-yq-4.49.2) is consistent with a conda-generated environment file and suitable for reproducible tests.

utils/go-yq/test/Snakefile (1)

1-79: Test Snakefile covers key go‑yq behaviors cleanly

The five rules nicely exercise concatenation, in-place updates, evaluating an embedded path expression, multi-output splitting (with explicit -s), and CSV→JSON conversion. Use of fixed paths and simple log filenames is in line with this repo’s test Snakefile conventions, so no changes are needed here.

Based on learnings, hardcoded IO and simplified logging in wrapper tests are acceptable.

utils/go-yq/wrapper.py (1)

54-77: Good use of expression quoting and command construction.

The use of shlex.quote() for the expression parameter is appropriate, and the shell command construction correctly handles subcommands, parameters, and logging.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9431b05 and 4cf5240.

📒 Files selected for processing (1)
  • utils/go-yq/wrapper.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py

⚙️ CodeRabbit configuration file

**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.

Files:

  • utils/go-yq/wrapper.py
**/wrapper.py

⚙️ CodeRabbit configuration file

Do not complain about use of undefined variable called snakemake.

Files:

  • utils/go-yq/wrapper.py
🧠 Learnings (19)
📓 Common learnings
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3502
File: bio/ngsbits/sampleancestry/wrapper.py:18-23
Timestamp: 2024-11-26T14:59:03.678Z
Learning: In Snakemake wrapper scripts, Snakemake validates input and output paths, so explicit shell quoting is not necessary.
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/preprocess-variants/wrapper.py:0-0
Timestamp: 2024-11-15T13:48:33.759Z
Learning: In Snakemake wrappers, security considerations like input sanitization are unnecessary, as the wrappers are under full control of the user.
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-21T10:23:03.427Z
Learning: In the Snakemake wrappers project, avoid suggesting extensive error handling or temporary file management in simple wrapper scripts when it may be unnecessary, to prevent overcomplicating the code.
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3123
File: utils/datavzrd/wrapper.py:31-32
Timestamp: 2024-10-08T17:41:54.542Z
Learning: The `snakemake` variable is inserted via a preamble during execution in `wrapper.py` scripts, so it doesn't need to be explicitly defined.
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3123
File: utils/datavzrd/wrapper.py:31-32
Timestamp: 2024-08-21T08:33:50.878Z
Learning: The `snakemake` variable is inserted via a preamble during execution in `wrapper.py` scripts, so it doesn't need to be explicitly defined.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/wrapper.py:27-28
Timestamp: 2024-11-26T08:30:23.818Z
Learning: In Snakemake wrappers (e.g., `wrapper.py` files), it's unnecessary to verify the availability of tools like `mtnucratio` within the code, because Snakemake with Conda ensures that the required tools are installed and available.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22
Timestamp: 2024-11-26T09:16:39.570Z
Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/test/Snakefile:58-71
Timestamp: 2024-11-26T10:49:54.765Z
Learning: In test Snakefiles within the snakemake-wrappers repository, it is acceptable to use simplified paths and logging configurations that may differ from real-life pipelines.
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 4285
File: bio/agat/wrapper.py:336-340
Timestamp: 2025-06-20T13:10:29.240Z
Learning: User tdayris prefers to fix identified code issues immediately rather than deferring them, as indicated by their comment "Now is better than later" when confirming code fixes.
📚 Learning: 2024-11-21T10:23:03.427Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-21T10:23:03.427Z
Learning: In the Snakemake wrappers project, avoid suggesting extensive error handling or temporary file management in simple wrapper scripts when it may be unnecessary, to prevent overcomplicating the code.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-26T14:59:03.678Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3502
File: bio/ngsbits/sampleancestry/wrapper.py:18-23
Timestamp: 2024-11-26T14:59:03.678Z
Learning: In Snakemake wrapper scripts, Snakemake validates input and output paths, so explicit shell quoting is not necessary.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-26T09:08:06.041Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/wrapper.py:18-24
Timestamp: 2024-11-26T09:08:06.041Z
Learning: In Snakemake wrappers, input file validation is managed by Snakemake, so manual validation in the wrapper code is unnecessary.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-10-08T17:41:54.542Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 3115
File: CHANGELOG.md:5-5
Timestamp: 2024-10-08T17:41:54.542Z
Learning: Do not review release-please commits in the Snakemake wrappers repository as they are auto-formatted.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-12-06T14:25:43.922Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3498
File: bio/ngscheckmate/ncm/wrapper.py:68-68
Timestamp: 2024-12-06T14:25:43.922Z
Learning: In the `bio/ngscheckmate/ncm/wrapper.py` file for the NGSCheckMate wrapper, do not suggest adding file existence checks for the list file input, as the files are already required by the wrapper.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-08-21T08:30:42.757Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3123
File: utils/datavzrd/wrapper.py:31-32
Timestamp: 2024-08-21T08:30:42.757Z
Learning: In `wrapper.py` scripts, do not flag the use of an undefined variable called `snakemake`.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-10-08T17:41:54.542Z
Learnt from: MagdalenaZZ
Repo: snakemake/snakemake-wrappers PR: 3071
File: bio/paraphase/wrapper.py:0-0
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In Snakemake wrappers, a try/except block is unnecessary because Snakemake will handle exceptions if the wrapper fails.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-26T09:16:39.570Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:14-22
Timestamp: 2024-11-26T09:16:39.570Z
Learning: In the `snakemake-wrappers` repository, when writing test `Snakefile`s (e.g., `bio/ngscheckmate/makesnvpattern/test/Snakefile`), hardcoded input/output paths are acceptable because these are examples and the IO can be chosen freely.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-26T08:31:00.099Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3496
File: bio/mtnucratio/test/Snakefile:2-6
Timestamp: 2024-11-26T08:31:00.099Z
Learning: In test files for Snakemake wrappers, such as `bio/mtnucratio/test/Snakefile`, hard-coded input and output paths are acceptable as examples and do not need to use wildcards to make paths flexible.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-15T18:31:15.447Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/test/Snakefile:7-10
Timestamp: 2024-11-15T18:31:15.447Z
Learning: In the Snakemake wrappers repository, avoid suggesting refactoring that involves using `tempfile.gettempdir()` or changing output paths to temporary directories.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-26T09:16:24.981Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3499
File: bio/ngscheckmate/makesnvpattern/test/Snakefile:1-13
Timestamp: 2024-11-26T09:16:24.981Z
Learning: In test `Snakefile`s (e.g., `test/Snakefile`), it's acceptable to use fixed input and output file names instead of wildcards.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2025-01-30T14:19:53.384Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 3432
File: bio/reference/ensembl-sequence/wrapper.py:79-87
Timestamp: 2025-01-30T14:19:53.384Z
Learning: In Snakemake wrappers, error reporting should use `print(..., file=sys.stderr)` for direct error messages, while `snakemake.log_fmt_shell()` is used for capturing command output and errors.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-26T10:49:54.765Z
Learnt from: tdayris
Repo: snakemake/snakemake-wrappers PR: 3501
File: meta/bio/varscan2_snpeff/test/Snakefile:58-71
Timestamp: 2024-11-26T10:49:54.765Z
Learning: In test Snakefiles within the snakemake-wrappers repository, it is acceptable to use simplified paths and logging configurations that may differ from real-life pipelines.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-15T13:44:18.810Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/estimate-alignment-properties/wrapper.py:5-12
Timestamp: 2024-11-15T13:44:18.810Z
Learning: In the snakemake-wrappers repository, it is acceptable to use f-strings with `shell=True` in wrapper scripts (e.g., `wrapper.py` files), as these wrappers are under full control of the user. Potential command injection vulnerabilities are not considered an issue in this context.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2025-06-15T07:43:03.263Z
Learnt from: tedil
Repo: snakemake/snakemake-wrappers PR: 4274
File: bio/mehari/annotate-seqvars/wrapper.py:11-19
Timestamp: 2025-06-15T07:43:03.263Z
Learning: In Snakemake wrappers, a common pattern is to have flag variables that are either empty strings "" or the actual flag strings (e.g., "--keep-intergenic") for direct interpolation in shell commands. This avoids conditionals in the shell expression and keeps the command construction clean.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-15T18:36:04.660Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/call-variants/wrapper.py:15-23
Timestamp: 2024-11-15T18:36:04.660Z
Learning: In the Snakemake wrappers repository, using `shell=True` and redirecting within shell commands is acceptable.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-09-04T18:46:15.788Z
Learnt from: dlaehnemann
Repo: snakemake/snakemake-wrappers PR: 3165
File: bio/nanosim/simulator/wrapper.py:1-163
Timestamp: 2024-09-04T18:46:15.788Z
Learning: In the `wrapper.py` script for the NanoSim Snakemake wrapper, the subcommand is inferred from the pattern of named input files, rather than being explicitly specified.

Applied to files:

  • utils/go-yq/wrapper.py
📚 Learning: 2024-11-15T13:48:33.759Z
Learnt from: johanneskoester
Repo: snakemake/snakemake-wrappers PR: 3478
File: bio/varlociraptor/preprocess-variants/wrapper.py:0-0
Timestamp: 2024-11-15T13:48:33.759Z
Learning: In Snakemake wrappers, security considerations like input sanitization are unnecessary, as the wrappers are under full control of the user.

Applied to files:

  • utils/go-yq/wrapper.py
🪛 Ruff (0.14.7)
utils/go-yq/wrapper.py

34-37: Avoid specifying long messages outside the exception class

(TRY003)


50-50: Undefined name snakemake

(F821)


51-51: Undefined name snakemake

(F821)


52-52: Undefined name snakemake

(F821)


54-54: Undefined name snakemake

(F821)


56-56: Undefined name snakemake

(F821)


59-59: Undefined name snakemake

(F821)


64-64: Undefined name snakemake

(F821)


65-65: Undefined name snakemake

(F821)


67-70: Avoid specifying long messages outside the exception class

(TRY003)


71-71: Undefined name snakemake

(F821)


73-73: Undefined name snakemake

(F821)


74-74: Undefined name snakemake

(F821)


75-75: Undefined name snakemake

(F821)


77-77: Avoid specifying long messages outside the exception class

(TRY003)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: testing
  • GitHub Check: docs
  • GitHub Check: Summary
🔇 Additional comments (3)
utils/go-yq/wrapper.py (3)

1-11: LGTM!

The imports and module setup are appropriate for a Snakemake wrapper.


79-79: LGTM!

The shell command structure is appropriate for invoking yq with the assembled parameters.


54-56: Verify that empty subcommand and '' (empty quoted expression) produce valid yq commands.

The web search indicates that yq can omit the expression argument entirely (defaulting to .), but does not explicitly confirm whether an empty quoted string '' is accepted as valid input. Similarly, the behavior of an empty subcommand parameter needs verification against yq's actual command-line syntax.

@fgvieira fgvieira merged commit 57e3fa8 into snakemake:master Dec 10, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants