fix: add default json function to benchmarks by fgvieira · Pull Request #4128 · snakemake/snakemake

fgvieira · 2026-03-24T10:13:00Z

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

Bug Fixes
- Improved benchmark output serialization so non-standard values are converted to strings instead of causing errors.
Tests
- Updated benchmark tests and expected results to reflect extended parameter contents in benchmark records.

coderabbitai · 2026-03-24T10:13:19Z

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)

CHANGELOG.md is excluded by !CHANGELOG.md

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b99575a9-d68e-469b-b119-4e1096b0aea5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

The change adds default=str to JSON serialization in BenchmarkRecord.to_json() so non-JSON-serializable benchmark values are stringified. Tests were updated to include additional non-standard params (a Path and a string) and expected JSONL output adjusted accordingly.

Changes

Cohort / File(s)	Summary
JSON Serialization Enhancement `src/snakemake/benchmark.py`	Added `default=str` to the `json.dumps()` call in `BenchmarkRecord.to_json()` so non-serializable benchmark values are converted to strings during serialization.
Test Updates `tests/test_benchmark_jsonl/Snakefile`, `tests/test_benchmark_jsonl/expected-results/test.benchmark_shell.jsonl`	Extended `bench_shell` rule `params` with `path = Path()` and `string = "foobar"`; updated expected JSONL to include these params serialized as strings/values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically identifies the main change: adding a default JSON function to handle non-JSON-serializable benchmark values.
Description check	✅ Passed	The description follows the template with all QC checkboxes properly checked and the AI-assistance section completed, confirming test coverage and manual code review.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ 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

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 `@tests/test_benchmark_jsonl/Snakefile`:
- Around line 19-20: The Snakefile uses Path() but never imports it, causing a
NameError when Snakemake parses the workflow; add an explicit import such as
"from pathlib import Path" at the top of the Snakefile so Path() is defined and
the test remains self-contained (look for the Path() usage in the
test_benchmark_jsonl/Snakefile).

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d019b40b-2605-4794-9a3f-712c317fa304

📥 Commits

Reviewing files that changed from the base of the PR and between 2432a6c and 35ad879.

📒 Files selected for processing (3)

src/snakemake/benchmark.py
tests/test_benchmark_jsonl/Snakefile
tests/test_benchmark_jsonl/expected-results/test.benchmark_shell.jsonl

🤖 I have created a release *beep* *boop* --- ## [9.21.1](v9.21.0...v9.21.1) (2026-05-29) ### Bug Fixes * add default json function to benchmarks ([#4128](#4128)) ([41fab22](41fab22)) * do not rerun when checkpoint job missing but downstream file exists ([#4124](#4124)) ([a060b93](a060b93)) * ensure that error logs contain all available details ([#4183](#4183)) ([74a86e9](74a86e9)) * handle missing pss attribute in benchmark on Windows ([#4160](#4160)) ([da52080](da52080)) * implement Resources.setdefault ([#3968](#3968)) ([2413e99](2413e99)) * reporting remote nodes number ([#3978](#3978)) ([8c534f0](8c534f0)) * resolve pathvars before constructing storage queries ([#3969](#3969)) ([bd15237](bd15237)) * storage temp() file cleanup with RemoteProvider ([#4189](#4189)) ([898bad1](898bad1)) * tolerate FileNotFoundError in drop_iocache ([#4153](#4153)) ([#4191](#4191)) ([ce26b28](ce26b28)) ### Documentation * Added guide on debugging workflows ([#4029](#4029)) ([3d052ae](3d052ae)) * **cli:** Remove broken ref bold markup ([#4204](#4204)) ([1200ebf](1200ebf)) * remove duplicated resources attribute in rules.rst ([#4190](#4190)) ([6c8ecdd](6c8ecdd)) * **rules:** Update script type hint advice ([#4193](#4193)) ([6108712](6108712)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

fgvieira added 3 commits March 24, 2026 11:10

Add default JSON function

03282c5

Update test

e6aa310

Update expected test results

35ad879

fgvieira requested a review from johanneskoester as a code owner March 24, 2026 10:13

fgvieira added this to Snakemake Hackathon 2026 Mar 24, 2026

fgvieira self-assigned this Mar 24, 2026

fgvieira added the bug Something isn't working label Mar 24, 2026

fgvieira moved this to In review in Snakemake Hackathon 2026 Mar 24, 2026

coderabbitai Bot reviewed Mar 24, 2026

View reviewed changes

Comment thread tests/test_benchmark_jsonl/Snakefile

fgvieira and others added 2 commits March 24, 2026 11:24

Add import

c68671c

Merge branch 'main' into bench_json_posixpath

6dba2a7

cademirch self-requested a review March 30, 2026 17:56

cademirch approved these changes Mar 30, 2026

View reviewed changes

fgvieira added 2 commits March 30, 2026 21:56

Fix typo

fbbaddf

Merge branch 'main' into bench_json_posixpath

58a697f

johanneskoester approved these changes May 29, 2026

View reviewed changes

johanneskoester merged commit 41fab22 into snakemake:main May 29, 2026
110 of 112 checks passed

github-project-automation Bot moved this from In review to Done in Snakemake Hackathon 2026 May 29, 2026

snakemake-bot mentioned this pull request May 29, 2026

chore(main): release 9.21.1 #4200

Merged

fgvieira deleted the bench_json_posixpath branch May 29, 2026 12:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add default json function to benchmarks#4128

fix: add default json function to benchmarks#4128
johanneskoester merged 7 commits into
snakemake:mainfrom
fgvieira:bench_json_posixpath

fgvieira commented Mar 24, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 24, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

fgvieira commented Mar 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

QC

AI-assistance disclosure

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fgvieira commented Mar 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 24, 2026 •

edited

Loading