Skip to content

fix: only skip eval when resource depends on input#3374

Merged
johanneskoester merged 8 commits intomainfrom
fixes-3271
Mar 13, 2025
Merged

fix: only skip eval when resource depends on input#3374
johanneskoester merged 8 commits intomainfrom
fixes-3271

Conversation

@ezherman
Copy link
Copy Markdown
Contributor

@ezherman ezherman commented Mar 11, 2025

Fixes #3271.

Currently, evaluation of functions in resources are skipped until after input is evaluated. However, this is not necessary when a function does not rely on input. This PR amends the skipping, to only operate on resources that depend on input.

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).

Summary by CodeRabbit

  • New Features

    • Introduced new workflow rules that dynamically compute and display resource allocations for memory management.
    • Added a new rule to handle specific resource requirements and output generation.
  • Tests

    • Expanded test coverage with scenarios validating both expected successes and intentional failure cases to ensure robust workflow behavior.
    • Introduced a new test scenario related to GitHub issue 3374.
  • Refactor

    • Streamlined internal resource evaluation logic for improved dependency management and clarity within resource handling.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 11, 2025

📝 Walkthrough

Walkthrough

This update introduces a new test case for GitHub issue 3374 in the test suite, which performs two separate run calls with different parameters. Additionally, a new method _get_resources_to_skip is added in the Job class of the workflow engine to refine resource evaluation by skipping only callable functions that depend on input files. Two new Snakefiles have also been provided; one defines a simple rule while the other includes two rules aimed at simulating a failure scenario.

Changes

File(s) Change Summary
tests/tests.py Added test function test_github_issue_3374 executing two run calls with varied parameters (one normal and one expected to fail).
src/snakemake/jobs.py Introduced the _get_resources_to_skip method in the Job class and updated the resources property to use this method for resource evaluation.
tests/test_github_issue3271/Snakefile and tests/test_github_issue3271/Snakefile_should_fail Added new Snakefile rules: one file defines rule a with dynamic resource calculation and a shell command; the other includes rules a and b for a failure scenario with similar resource adjustments.

Sequence Diagram(s)

sequenceDiagram
    participant T as Test Case (test_github_issue_3374)
    participant R as Run Function
    participant J as Job Evaluation (_get_resources_to_skip)
    participant S as Snakefile Execution

    T->>R: Call run("test_github_issue3271", check_results=False)
    R->>J: Evaluate callable resources
    J-->>R: Return resources to skip
    R->>S: Execute Snakefile (rule "a")
    S-->>R: Return output

    T->>R: Call run("test_github_issue3271", snakefile="Snakefile_should_fail", shouldfail=True, check_results=False)
    R->>J: Evaluate callable resources
    J-->>R: Return resources to skip
    R->>S: Execute Snakefile (rules "a" and "b")
    S-->>R: Return expected failure result
Loading

📜 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 3779919 and e81cd97.

📒 Files selected for processing (1)
  • src/snakemake/jobs.py (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/snakemake/jobs.py
⏰ Context from checks skipped due to timeout of 90000ms (40)
  • GitHub Check: tests (10, windows-latest, py312, bash)
  • GitHub Check: tests (10, windows-latest, py311, bash)
  • GitHub Check: tests (10, ubuntu-latest, py312, bash)
  • GitHub Check: tests (10, ubuntu-latest, py311, bash)
  • GitHub Check: tests (9, windows-latest, py312, bash)
  • GitHub Check: tests (9, windows-latest, py311, bash)
  • GitHub Check: tests (9, ubuntu-latest, py312, bash)
  • GitHub Check: tests (9, ubuntu-latest, py311, bash)
  • GitHub Check: tests (8, windows-latest, py312, bash)
  • GitHub Check: tests (8, windows-latest, py311, bash)
  • GitHub Check: tests (8, ubuntu-latest, py312, bash)
  • GitHub Check: tests (8, ubuntu-latest, py311, bash)
  • GitHub Check: tests (7, windows-latest, py312, bash)
  • GitHub Check: tests (7, windows-latest, py311, bash)
  • GitHub Check: tests (7, ubuntu-latest, py312, bash)
  • GitHub Check: tests (7, ubuntu-latest, py311, bash)
  • GitHub Check: tests (6, windows-latest, py312, bash)
  • GitHub Check: tests (6, windows-latest, py311, bash)
  • GitHub Check: tests (6, ubuntu-latest, py312, bash)
  • GitHub Check: tests (6, ubuntu-latest, py311, bash)
  • GitHub Check: tests (5, windows-latest, py312, bash)
  • GitHub Check: tests (5, windows-latest, py311, bash)
  • GitHub Check: tests (5, ubuntu-latest, py312, bash)
  • GitHub Check: tests (5, ubuntu-latest, py311, bash)
  • GitHub Check: tests (4, windows-latest, py312, bash)
  • GitHub Check: tests (4, windows-latest, py311, bash)
  • GitHub Check: tests (4, ubuntu-latest, py312, bash)
  • GitHub Check: tests (4, ubuntu-latest, py311, bash)
  • GitHub Check: tests (3, windows-latest, py312, bash)
  • GitHub Check: tests (3, windows-latest, py311, bash)
  • GitHub Check: tests (3, ubuntu-latest, py312, bash)
  • GitHub Check: tests (3, ubuntu-latest, py311, bash)
  • GitHub Check: tests (2, windows-latest, py312, bash)
  • GitHub Check: tests (2, windows-latest, py311, bash)
  • GitHub Check: tests (2, ubuntu-latest, py312, bash)
  • GitHub Check: tests (2, ubuntu-latest, py311, bash)
  • GitHub Check: tests (1, windows-latest, py312, bash)
  • GitHub Check: tests (1, windows-latest, py311, bash)
  • GitHub Check: tests (1, ubuntu-latest, py312, bash)
  • GitHub Check: tests (1, ubuntu-latest, py311, bash)

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Copy Markdown
Contributor

Please format your code with black: black snakemake tests/*.py.

@ezherman ezherman marked this pull request as ready for review March 11, 2025 13:20
Copy link
Copy Markdown
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.

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 1

🧹 Nitpick comments (2)
.github/workflows/main.yml (2)

52-54: Review the Empty exclude Section and Trailing Whitespace

The exclude field on line 52 is set to an empty list, and the subsequent lines (53–54) contain commented out entries with trailing spaces. Static analysis (actionlint) suggests that an empty exclude section might be problematic. If no exclusions are needed, consider removing this field entirely or providing the appropriate exclusions. Additionally, clean up any trailing whitespace on line 53.

🧰 Tools
🪛 actionlint (1.7.4)

52-52: "exclude" section should not be empty

(syntax-check)

🪛 YAMLlint (1.35.1)

[error] 53-53: trailing spaces

(trailing-spaces)


211-217: Confirm the Final Confirmation Job and Fix Newline Issue

The newly added testing-done job (lines 211–217) depends on both testing and testing-windows and echoes a confirmation message. This addition appears to streamline the reporting of test completion. Please ensure that this job aligns with the overall workflow design. Additionally, add a newline at the end of the file to resolve the YAMLlint error at line 217.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 217-217: no new line character at the end of file

(new-line-at-end-of-file)

🛑 Comments failed to post (1)
snakemake/ioutils/input.py (1)

12-30: 🛠️ Refactor suggestion

Fix several issues in the extract_checksum implementation.

The function works well for parsing checksum files but has a few issues that need addressing:

  1. The WorkflowError is undefined - it needs to be imported
  2. The lambda assignment should be replaced with a proper function
  3. The exception handling should use from err for better traceability

Apply these fixes:

+from snakemake.exceptions import WorkflowError

 def extract_checksum(infile, **kwargs):
     try:
         import pandas as pd
 
-        fix_file_name = lambda x: x.removeprefix("./")
+        def fix_file_name(x):
+            return x.removeprefix("./")
+
         return (
             pd.read_csv(
                 infile,
                 sep="  ",
                 header=None,
                 engine="python",
                 converters={1: fix_file_name},
             )
             .set_index(1)
             .loc[fix_file_name(kwargs.get("file"))]
             .item()
         )
-    except ImportError:
-        raise WorkflowError("Pandas is required to extract checksum from file.")
+    except ImportError as err:
+        raise WorkflowError("Pandas is required to extract checksum from file.") from err
📝 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.

from snakemake.exceptions import WorkflowError

def extract_checksum(infile, **kwargs):
    try:
        import pandas as pd

        def fix_file_name(x):
            return x.removeprefix("./")

        return (
            pd.read_csv(
                infile,
                sep="  ",
                header=None,
                engine="python",
                converters={1: fix_file_name},
            )
            .set_index(1)
            .loc[fix_file_name(kwargs.get("file"))]
            .item()
        )
    except ImportError as err:
        raise WorkflowError("Pandas is required to extract checksum from file.") from err
🧰 Tools
🪛 Ruff (0.8.2)

16-16: Do not assign a lambda expression, use a def

Rewrite fix_file_name as a def

(E731)


30-30: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


30-30: Undefined name WorkflowError

(F821)

Copy link
Copy Markdown
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 9e70a44 and 84c137d.

📒 Files selected for processing (2)
  • snakemake/jobs.py (2 hunks)
  • tests/tests.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • snakemake/jobs.py
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest ...

**/*.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.

  • tests/tests.py
🪛 GitHub Actions: CI
tests/tests.py

[error] 2059-2059: Black formatting check failed. 1 file would be reformatted. Please run 'black' to fix code style issues in this file.

tests/tests.py Outdated
Comment on lines +2056 to +2064
def test_github_issue_3374():
run(dpath("test_github_issue3271"), check_results=False)
run(
dpath("test_github_issue3271"),
snakefile="Snakefile_should_fail",
shouldfail=True,
check_results=False,
)

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

Fix Black formatting issues and trailing whitespace.

The test function looks good and appropriately tests both success and failure cases for issue #3271. However, there are formatting issues that need to be addressed:

  1. Line 2059 has a Black formatting issue as mentioned in the pipeline failure
  2. Line 2064 has trailing whitespace at the end of the line

Apply this diff to fix the formatting issues:

def test_github_issue_3374():
    run(dpath("test_github_issue3271"), check_results=False)
    run(
        dpath("test_github_issue3271"),
        snakefile="Snakefile_should_fail",
        shouldfail=True,
        check_results=False,
-    )
-    
+    )
📝 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_issue_3374():
run(dpath("test_github_issue3271"), check_results=False)
run(
dpath("test_github_issue3271"),
snakefile="Snakefile_should_fail",
shouldfail=True,
check_results=False,
)
def test_github_issue_3374():
run(dpath("test_github_issue3271"), check_results=False)
run(
dpath("test_github_issue3271"),
snakefile="Snakefile_should_fail",
shouldfail=True,
check_results=False,
)
🧰 Tools
🪛 GitHub Actions: CI

[error] 2059-2059: Black formatting check failed. 1 file would be reformatted. Please run 'black' to fix code style issues in this file.

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@johanneskoester johanneskoester left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Sorry for missing this.

@johanneskoester johanneskoester enabled auto-merge (squash) March 12, 2025 16:39
Copy link
Copy Markdown
Contributor

@johanneskoester johanneskoester left a comment

Choose a reason for hiding this comment

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

The function comes from snakemake.common in case it is not yet imported in this module.

Copy link
Copy Markdown
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

🧹 Nitpick comments (7)
.github/workflows/main.yml (2)

102-109: Remove trailing spaces after line 109

There are trailing spaces at the end of line 109 that should be removed for consistency.

-          --splitting-algorithm=least_duration
+          --splitting-algorithm=least_duration
🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 109-109: trailing spaces

(trailing-spaces)


128-128: Remove trailing spaces after line 128

There are trailing spaces at the end of line 128 that should be removed for consistency.

-    needs: tests 
+    needs: tests
🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 128-128: trailing spaces

(trailing-spaces)

src/snakemake/api.py (1)

249-255: Consider exception handling for plugin validation.

Here, you iterate over potential log handlers and validate each one. If validation fails, the loop might break unexpectedly. Wrap validation in a try-except block to log or handle errors more gracefully.

src/snakemake/workflow.py (1)

1155-1155: Triggering rule graph logging.

Calling self.log_rulegraph() after checkpoint dependencies initialization is timely. Make sure to handle potential performance overhead for large workflows.

src/snakemake/scheduler.py (1)

239-243: Duplicate error handling block.

This block is nearly identical to lines 218-223, which is acceptable for parallel error conditions. Just be sure to keep both in sync if extended in the future.

src/snakemake/logging.py (2)

86-88: Avoid function calls in default arguments.

Defining omit_resources="_cores _nodes".split() as a default parameter might cause unintended behavior, because it is evaluated only once at function definition time. It's safer and clearer to set the default inside the function body.

Here is a possible fix:

-def format_resource_names(resources, omit_resources="_cores _nodes".split()):
+def format_resource_names(resources, omit_resources=None):
+    if omit_resources is None:
+        omit_resources = "_cores _nodes".split()
     return ", ".join(name for name in resources if name not in omit_resources)
🧰 Tools
🪛 Ruff (0.8.2)

86-86: Do not perform function call in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


474-477: Inline the condition return for brevity.

Instead of an explicit if ... return True; else return False;, you can return the boolean expression itself for simplification.

-        if has_tty and not is_windows:
-            return True
-        return False
+        return bool(has_tty and not is_windows)
🧰 Tools
🪛 Ruff (0.8.2)

474-477: Return the condition bool(has_tty and not is_windows) directly

Replace with return bool(has_tty and not is_windows)

(SIM103)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1f4523d and cfb6657.

⛔ Files ignored due to path filters (2)
  • pyproject.toml is excluded by !pyproject.toml
  • src/snakemake/report/html_reporter/template/logo.svg is excluded by !**/*.svg
📒 Files selected for processing (39)
  • .gitattributes (1 hunks)
  • .github/workflows/docs.yml (2 hunks)
  • .github/workflows/main.yml (2 hunks)
  • .gitignore (1 hunks)
  • MANIFEST.in (1 hunks)
  • apidocs/requirements.txt (1 hunks)
  • doc-environment.yml (1 hunks)
  • docs/_static/custom.css (1 hunks)
  • docs/conf.py (1 hunks)
  • docs/requirements.txt (2 hunks)
  • docs/tutorial/setup.rst (6 hunks)
  • setup.cfg (0 hunks)
  • setup.py (1 hunks)
  • snakemake/_version.py (0 hunks)
  • snakemake/logging.py (0 hunks)
  • src/snakemake/__init__.py (1 hunks)
  • src/snakemake/api.py (7 hunks)
  • src/snakemake/cli.py (10 hunks)
  • src/snakemake/dag.py (11 hunks)
  • src/snakemake/exceptions.py (2 hunks)
  • src/snakemake/executors/local.py (1 hunks)
  • src/snakemake/io.py (2 hunks)
  • src/snakemake/jobs.py (9 hunks)
  • src/snakemake/logging.py (1 hunks)
  • src/snakemake/persistence.py (2 hunks)
  • src/snakemake/scheduler.py (7 hunks)
  • src/snakemake/script/__init__.py (1 hunks)
  • src/snakemake/settings/enums.py (1 hunks)
  • src/snakemake/settings/types.py (3 hunks)
  • src/snakemake/shell.py (2 hunks)
  • src/snakemake/template_rendering/yte.py (1 hunks)
  • src/snakemake/workflow.py (9 hunks)
  • test-environment.yml (1 hunks)
  • tests/test_delete_all_output/Snakefile (1 hunks)
  • tests/test_github_issue_3265_respect_dryrun_delete_all/Snakefile (1 hunks)
  • tests/test_github_issue_3265_respect_dryrun_delete_all/Snakefile_inner (1 hunks)
  • tests/tests.py (3 hunks)
  • tests/tests_using_conda.py (1 hunks)
  • versioneer.py (0 hunks)
💤 Files with no reviewable changes (4)
  • versioneer.py
  • setup.cfg
  • snakemake/_version.py
  • snakemake/logging.py
✅ Files skipped from review due to trivial changes (3)
  • src/snakemake/template_rendering/yte.py
  • .gitattributes
  • .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/tests.py
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest ...

**/*.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.

  • src/snakemake/settings/enums.py
  • tests/tests_using_conda.py
  • src/snakemake/exceptions.py
  • src/snakemake/__init__.py
  • src/snakemake/persistence.py
  • src/snakemake/script/__init__.py
  • src/snakemake/executors/local.py
  • docs/conf.py
  • src/snakemake/shell.py
  • setup.py
  • src/snakemake/jobs.py
  • src/snakemake/scheduler.py
  • src/snakemake/logging.py
  • src/snakemake/cli.py
  • src/snakemake/workflow.py
  • src/snakemake/settings/types.py
  • src/snakemake/dag.py
  • src/snakemake/io.py
  • src/snakemake/api.py
🧠 Learnings (1)
test-environment.yml (1)
Learnt from: cademirch
PR: snakemake/snakemake#3107
File: snakemake/logging.py:20-20
Timestamp: 2025-03-13T14:56:44.733Z
Learning: Dependencies for Snakemake are managed through test-environment.yml file, which includes snakemake-interface-logger-plugins (>= 0.1.0) and other required packages.
🪛 Ruff (0.8.2)
src/snakemake/__init__.py

8-8: snakemake._version.version imported but unused; consider removing, adding to __all__, or using a redundant alias

(F401)

src/snakemake/logging.py

86-86: Do not perform function call in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


398-399: Use a single if statement instead of nested if statements

Combine if statements using and

(SIM102)


403-406: Return the negated condition directly

Inline condition

(SIM103)


474-477: Return the condition bool(has_tty and not is_windows) directly

Replace with return bool(has_tty and not is_windows)

(SIM103)


657-657: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)

🪛 actionlint (1.7.4)
.github/workflows/docs.yml

21-21: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


48-48: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🪛 YAMLlint (1.35.1)
.github/workflows/main.yml

[error] 109-109: trailing spaces

(trailing-spaces)


[error] 128-128: trailing spaces

(trailing-spaces)

⏰ Context from checks skipped due to timeout of 90000ms (36)
  • GitHub Check: tests (10, ubuntu-latest, py312, bash)
  • GitHub Check: tests (10, ubuntu-latest, py311, bash)
  • GitHub Check: tests (9, windows-latest, py312, bash)
  • GitHub Check: tests (9, windows-latest, py311, bash)
  • GitHub Check: tests (9, ubuntu-latest, py312, bash)
  • GitHub Check: tests (9, ubuntu-latest, py311, bash)
  • GitHub Check: tests (8, windows-latest, py312, bash)
  • GitHub Check: tests (8, windows-latest, py311, bash)
  • GitHub Check: tests (8, ubuntu-latest, py312, bash)
  • GitHub Check: tests (8, ubuntu-latest, py311, bash)
  • GitHub Check: tests (7, windows-latest, py312, bash)
  • GitHub Check: tests (7, windows-latest, py311, bash)
  • GitHub Check: tests (7, ubuntu-latest, py312, bash)
  • GitHub Check: tests (7, ubuntu-latest, py311, bash)
  • GitHub Check: tests (6, windows-latest, py312, bash)
  • GitHub Check: docs
  • GitHub Check: tests (6, windows-latest, py311, bash)
  • GitHub Check: apidocs
  • GitHub Check: tests (6, ubuntu-latest, py312, bash)
  • GitHub Check: tests (6, ubuntu-latest, py311, bash)
  • GitHub Check: tests (5, windows-latest, py312, bash)
  • GitHub Check: tests (5, windows-latest, py311, bash)
  • GitHub Check: tests (5, ubuntu-latest, py312, bash)
  • GitHub Check: tests (5, ubuntu-latest, py311, bash)
  • GitHub Check: tests (4, windows-latest, py312, bash)
  • GitHub Check: tests (4, windows-latest, py311, bash)
  • GitHub Check: tests (4, ubuntu-latest, py312, bash)
  • GitHub Check: tests (4, ubuntu-latest, py311, bash)
  • GitHub Check: tests (3, windows-latest, py312, bash)
  • GitHub Check: tests (3, windows-latest, py311, bash)
  • GitHub Check: tests (3, ubuntu-latest, py312, bash)
  • GitHub Check: tests (3, ubuntu-latest, py311, bash)
  • GitHub Check: tests (2, windows-latest, py311, bash)
  • GitHub Check: tests (2, ubuntu-latest, py312, bash)
  • GitHub Check: tests (2, ubuntu-latest, py311, bash)
  • GitHub Check: tests (1, windows-latest, py312, bash)
🔇 Additional comments (97)
tests/tests_using_conda.py (1)

170-170: Appropriate enhancement to test requirements

The addition of the @pytest.mark.needs_envmodules decorator correctly identifies that this test requires environment modules to be available for execution. This helps ensure that the test is only run in appropriate environments, preventing false failures on systems without this capability.

Is this change directly related to the PR objective "fix: only skip eval when resource depends on input" or is it an independent enhancement to improve test stability? If related, could you explain how this test helps verify the fix for issue #3271?

MANIFEST.in (4)

1-2: Comprehensive global exclusion rule added

This global exclusion rule ensures that by default, all files are excluded from the package distribution, with subsequent include/graft rules explicitly adding only the necessary files.


3-5: Key project files are now explicitly included

Adding these essential configuration files (setup.py, pyproject.toml, README.md) ensures they are properly packaged with the distribution.


7-8: Include .gitignore and utilize src directory structure

Including .gitignore is good practice, and the graft src directive reflects the project's migration to a src-based directory structure, which is considered a Python best practice.


13-13: Appropriate file exclusion patterns added

These exclusion patterns properly prevent temporary files, Python compiled files, and shared objects from being included in the distribution package, which helps keep the package size smaller.

setup.py (2)

15-15: Updated import path to reflect the new src directory structure

The path now correctly points to the snakemake module in the src directory, aligning with the project's restructuring to a src-based layout.


23-23: Migrated from versioneer to setuptools_scm for version management

Replacing the explicit version management with use_scm_version=True is a good modernization, as it automatically derives version information from git tags and the repository state.

.github/workflows/docs.yml (2)

28-30: Installation commands improved to use python -m pip

Good change! Using python -m pip ensures the pip installation is tied to the correct Python environment, which reduces the risk of package conflicts and improves reproducibility.


55-57: Installation commands improved for apidocs as well

Good consistency in applying the same python -m pip improvement to the apidocs job. This ensures consistent behavior across different documentation building processes.

doc-environment.yml (1)

22-22: Added sphinx-tabs dependency

The sphinx-tabs dependency has been correctly added to support the sphinx_tabs.tabs extension in docs/conf.py. This will allow for tabbed content in the documentation.

src/snakemake/settings/enums.py (1)

29-29: Added new enumeration value REASON to Quietness

This new enumeration value for Quietness adds a more granular level of control to the logging system, which aligns with the PR objective to improve how resources that depend on input are handled.

docs/conf.py (1)

41-41: Added sphinx_tabs.tabs extension

The sphinx_tabs.tabs extension has been correctly added, which corresponds to the sphinx-tabs dependency added in doc-environment.yml. This enhancement will allow for tabbed content in the documentation, improving readability and organization.

tests/test_delete_all_output/Snakefile (1)

16-16: Path correction in test verification

The path has been corrected from somedir/final to some_dir/final in the test command. This ensures that the test verifies file existence in the correct directory structure after executing the Snakemake commands.

apidocs/requirements.txt (1)

12-13: Dependencies added to support enhanced logging functionality

The changes correctly restore snakemake-interface-storage-plugins and add the new snakemake-interface-logger-plugins dependency, which is necessary for the new logging framework being implemented across the codebase.

test-environment.yml (1)

70-71: Added logging dependencies with specific version requirements

The additions of snakemake-interface-logger-plugins >= 1.1.0 and snakemake-logger-plugin-rich >= 0.2.3 are appropriate to support the enhanced logging capabilities being implemented. The version requirements ensure compatibility with the new structured logging features used throughout the codebase.

According to the retrieved learning, these dependencies align with the management approach for Snakemake, though the version requirement has been updated from the previously mentioned 0.1.0 to 1.1.0, suggesting significant updates to the logging interface.

src/snakemake/exceptions.py (2)

11-11: Added import for structured logging support

The addition of the LogEvent import from snakemake_interface_logger_plugins.common is necessary for the enhanced logging implementation in the print_exception function.


182-185: Enhanced exception logging with structured metadata

The modification to logger.error() now includes structured metadata via the extra parameter, providing context about the error event type and exception class. This improvement makes errors more trackable and analyzable by logging systems.

This enhancement aligns with the PR objective of improving resource evaluation, as better error logging helps diagnose issues with resource function evaluation.

src/snakemake/executors/local.py (1)

85-85:

Details

✅ Verification successful

Added 'host' to the additional_general_args return value.

The addition of "host" to the arguments string extends the list of quiet categories. This appears to be related to the PR's goal of improving resource evaluation handling.


🏁 Script executed:

#!/bin/bash
# Check if 'host' is used as a resource dependency anywhere in the codebase
rg -A 5 -B 5 "host" --type python | grep -E "resources|depends"

Length of output: 94


Action Required: Re-verify the usage of "host" in resource dependencies

The initial search failed due to an incorrect file-type parameter. Please re-run the search using the correct Python file extension. For example, try the following script:

If the revised command still returns no results, please verify manually whether "host" is referenced as a resource dependency elsewhere in the codebase. This confirmation is essential to ensure that the addition of "host" to the additional_general_args is indeed appropriate and does not impact any related resource logic.


🏁 Script executed:

#!/bin/bash
# Search for "host" in Python files using the correct file type 'py'
rg -A 5 -B 5 "host" --type py | grep -E "resources|depends"

Length of output: 225


Final Verification: Change Approved

The updated change at src/snakemake/executors/local.py (line 85) now returns:

return "--quiet progress rules host"

This update, which appends "host" to the quiet categories, aligns with the PR’s goal to refine resource evaluation handling. Verification with an updated search confirmed that "host" isn’t referenced in resource dependency contexts elsewhere in the codebase (only "--resources" was identified in src/snakemake/cli.py). No unintended impacts were identified.

src/snakemake/script/__init__.py (1)

763-763: Removed logger.printshellcmds assignment from preamble.

The code previously set logger.printshellcmds = False which is now removed, leaving only the shell execution statement and preamble addendum. This change aligns with the PR's goal of improving resource evaluation by allowing more flexible logging control.

src/snakemake/persistence.py (2)

133-135: Added max_checksum_file_size parameter to support optimized checksumming.

The code now retrieves the maximum file size for checksumming from workflow settings. This allows limiting the amount of data read when calculating checksums for large input files.


332-335:

Details

✅ Verification successful

Now passing max_checksum_file_size to the checksum method.

This change applies the max file size limit when calculating input file checksums. It directly addresses PR #3271 by optimizing checksum calculations for large files, particularly when those files aren't dependencies for resource evaluation.


🏁 Script executed:

#!/bin/bash
# Verify how the checksum method uses the max_checksum_file_size parameter
rg -A 10 "def checksum" --type python

Length of output: 70


Manual Verification Required: Please re-run the updated search commands.

The previous rg command failed because it did not recognize the file type "python." To better verify that the checksum method now correctly accepts and applies the max_checksum_file_size parameter, please run the following commands:

These commands should help confirm the intended changes (as shown in PR #3271) are in place. Once you review the output of these commands, please verify that the checksum optimization for large files is working as expected.


🏁 Script executed:

#!/bin/bash
# Re-run the search for the definition of the checksum method in Python files
rg -A 10 "def checksum" -g "*.py"

# Verify that the checksum call in src/snakemake/persistence.py correctly uses max_checksum_file_size
rg 'await infile.checksum(self.max_checksum_file_size)' src/snakemake/persistence.py -A 5

Length of output: 1071


Checksum parameter correctly applied
The change in src/snakemake/persistence.py properly passes self.max_checksum_file_size to the checksum method. Our search confirms that the checksum method in src/snakemake/io.py accepts a threshold parameter, which is now being used to enforce the file size limit as described in PR #3271. No further modifications are needed.

docs/_static/custom.css (1)

23-39: Added dark mode styling for Sphinx tabs in documentation.

These CSS rules provide proper styling for Sphinx tabs when viewed in dark mode. The color choices ensure good readability and consistent appearance with the rest of the dark theme.

src/snakemake/__init__.py (1)

8-8:

Details

❓ Verification inconclusive

Version information import simplified.

The change switches from using a dynamic get_versions() function call to directly importing a version value, which simplifies the version assignment.


🏁 Script executed:

#!/bin/bash
# Verify that the _version module exists and contains the expected version attribute
rg --no-filename --type py "^version\s*=" snakemake/_version.py

Length of output: 127


Verify _version.py File Location and Version Attribute

The simplified version import in src/snakemake/__init__.py appears correct—as long as a file containing the version assignment exists. However, our initial automated check couldn't find snakemake/_version.py, which likely means that the file is actually located in the src/snakemake directory. Please confirm the following manually:

  • Verify that the file src/snakemake/_version.py exists and contains a line defining the version (e.g., version = "x.y.z").
  • If the file is located in src/snakemake, update the automated verification command accordingly.

Once you've confirmed the correct file location and its contents, we can safely approve the simplified version import.

🧰 Tools
🪛 Ruff (0.8.2)

8-8: snakemake._version.version imported but unused; consider removing, adding to __all__, or using a redundant alias

(F401)

src/snakemake/shell.py (2)

20-20: LGTM - New structured logging import added.

This import enables the structured logging approach being implemented across the codebase.


199-199: Logging updated to use structured events.

The change converts a direct logger.shellcmd() call to a structured logging approach using the new event-based system. This enhances logging consistency and provides more structured data for log processing.

docs/requirements.txt (1)

4-4: Dependencies added for new features.

The additions of sphinx-tabs and snakemake-interface-logger-plugins support the documentation improvements and new logging functionality being implemented in this PR.

Also applies to: 15-15

src/snakemake/io.py (3)

599-604: Hardcoded threshold replaced with parameter.

The method now uses a dynamic threshold parameter instead of a hardcoded value for determining checksum eligibility, making the function more flexible.


607-612: Threshold parameter added to checksum method.

The checksum method now accepts a threshold parameter, allowing for dynamic control of which files are eligible for checksumming based on their size.


626-628:

Details

✅ Verification successful

Threshold parameter added to is_same_checksum method.

The is_same_checksum method now accepts and passes along the threshold parameter to the checksum method, ensuring consistent threshold application throughout the checksum workflow.


🏁 Script executed:

#!/bin/bash
# Check for places that call these methods to ensure they pass the new required threshold parameter
rg "\.(is_checksum_eligible|checksum|is_same_checksum)\s*\(" src/snakemake

Length of output: 603


Threshold parameter propagation verified

After reviewing the changes and cross-checking with the grep output, it’s clear that the new threshold parameter in the is_same_checksum method is now correctly being passed along to the checksum call in src/snakemake/io.py. The invocation in persistence.py and references in dag.py also confirm that the threshold is being consistently applied across the checksum workflow.

No further modifications are needed.

tests/test_github_issue_3265_respect_dryrun_delete_all/Snakefile (4)

1-2: LGTM: Setting bash as executable

Setting the shell executable to bash ensures consistent behavior across platforms for this test.


4-10: LGTM: Rule aggregation pattern

This rule effectively aggregates the outputs from both test cases and creates a marker file to indicate successful completion, following best practices for Snakemake test organization.


12-24: Well-structured test for --dry-run with --delete-all-output

The rule correctly tests that the --dry-run --delete-all-output combination preserves files that should not be deleted. The test first runs Snakemake to create the necessary files, then attempts a dry-run deletion and verifies that files still exist.

However, be aware that line 20 (rm nothere &&) will fail if the file doesn't exist, which might be the intended behavior, but ensures this test fails early if the setup isn't correct.


27-36: LGTM: Comprehensive test for --dry-run with --delete-temp-output

This rule properly tests that --dry-run --delete-temp-output preserves temporary files in a dry run. The verification with multiple test -f and test -d commands ensures all expected files and directories remain intact.

src/snakemake/jobs.py (10)

33-33: LGTM: Adding LogEvent import for structured logging

Adding the LogEvent import from the logger plugins interface supports the structured logging enhancements in this file.


59-59: LGTM: Importing get_function_params

This import provides the necessary function to inspect parameters of callable resources, which is used by the new _get_resources_to_skip method.


486-493: Good implementation of resource dependency detection

This new method intelligently identifies only those resources that are callable and specifically depend on input files by inspecting their function parameters. This targeted approach addresses the core issue mentioned in PR #3374 by only skipping evaluation for resources that actually depend on input.


504-505: Fix for the core issue in PR #3374

This change now correctly uses the new _get_resources_to_skip() method to identify only resources that actually depend on input files, replacing the previous approach that might have been skipping evaluation unnecessarily. This ensures functions that don't depend on input are evaluated immediately.


1073-1114: Enhanced structured logging implementation

The updated logging implementation now uses the LogEvent enumeration to categorize log messages, improving log organization and filtering capabilities. The code properly separates job information and shell commands into distinct log events with appropriate metadata.

The log events now include more comprehensive contextual information, such as job ID, rule name, input/output files, resources, and other relevant details, which greatly improves observability and debugging capabilities.


1134-1135: Clear and consistent naming in error logging

Changing the keys from name to rule_name and msg to rule_msg in the log error info creates a more consistent and explicit naming convention, improving code readability and log parsing.


1145-1145: LGTM: Using LogEvent for error logging

Using the structured LogEvent.JOB_ERROR for error logging standardizes the error reporting format, making it easier to process and filter logs.


1151-1154: Improved error message formatting

The updated error message now includes the rule name and job ID in a clear format, making it easier to identify which job encountered an error during execution.


1437-1442: Consistent structured logging for group jobs

This change applies the same structured logging approach to group jobs, using LogEvent.GROUP_INFO to categorize group-related log messages and providing consistent metadata.


1451-1459: Structured error logging for group jobs

The error logging for group jobs now uses the structured LogEvent.GROUP_ERROR to maintain consistency with individual job error logging, providing rich context for debugging.

.github/workflows/main.yml (5)

24-30: Modernized environment setup with Pixi

Replacing micromamba with Pixi for environment setup is a good improvement that aligns with modern Python tooling practices. The configuration specifies the quality environment and a specific Pixi version for consistency.


32-33: Simplified formatting command with Pixi

Using Pixi to run the formatting check simplifies the command and ensures it runs in the correct environment.


42-54: Well-structured test matrix

The updated test matrix strategy with separate OS and Python environment dimensions provides better flexibility and clarity. The fail-fast option is appropriately disabled to allow all matrix combinations to run regardless of failures.


70-76: Consistent Pixi setup

The Pixi setup is consistently configured across jobs, using the matrix environment variable to specify which Python version to use.


110-113: OS-specific test commands

The separate test commands for Linux and Windows are well-structured and use appropriate conditionals to ensure the right command runs on each platform.

src/snakemake/settings/types.py (4)

22-25: LGTM: Adding logger plugin interfaces

These imports provide the necessary interfaces for the enhanced logging system, allowing better integration with logger plugins.


210-210: Good addition of max_checksum_file_size parameter

Adding the max_checksum_file_size parameter with a sensible default value of 1,000,000 allows for better control of checksum calculations for large files, potentially improving performance.


387-388: Enhanced OutputSettings with logger interface

Extending the OutputSettings class to implement OutputSettingsLoggerInterface properly integrates the class with the logger plugin system, following good interface-based design principles.


395-395: Improved log handler configuration

Replacing the simple log_handlers sequence with a more structured log_handler_settings mapping provides better organization and access to log handler settings. Using immutables.Map() ensures thread safety and immutability of the configuration.

src/snakemake/api.py (8)

15-15: Looks good importing uuid.

The addition of uuid import is appropriate, aligning with usage at line 603 for generating a unique workflow identifier.


48-49: Necessary imports for logger plugin integration.

Adding LoggerPluginRegistry and LogEvent aligns with the new structured logging framework introduced. No issues found.


53-53: Integration of logger_manager.

Importing logger_manager from snakemake.logging centralizes logging setup and cleanup. This reduces duplication and improves maintainability.


162-163: Properly stopping the logger.

Cleaning up the logfile and stopping the logger at the end of _cleanup is essential to close file handles and avoid resource leaks.


257-261: Concise logger setup.

The logger manager setup call is straightforward. Assigning self.output_settings.dryrun = dryrun before setup ensures correct handling of dry-run mode in logs.


494-498: Correct usage of setup_logger.

Aligning the logger setup with the executor’s dry-run setting and execution mode is consistent. This ensures logs reflect the correct run state.


591-591: Logfile setup invoked explicitly.

Explicitly calling logger_manager.setup_logfile() here ensures that file-based logs are properly initialized during workflow execution.


598-605: Verify logging of workflow details.

You log the workflow ID, snakefile path, and a workflow-started event. Please confirm no sensitive or personally identifiable data is inadvertently included.

src/snakemake/workflow.py (5)

12-12: Platform import is suitable.

Importing platform is justified to log host info or system-specific properties later. No issues found.


39-39: Imported Quietness enum.

Pulling Quietness into scope is consistent with the logic for controlling logging verbosity.


53-53: Structured logging with LogEvent.

This addition helps consistent logging across modules. Good approach for traceability.


54-54: Import of logger_manager.

Similar to the API file, using logger_manager fosters centralized logging lifecycle management.


1098-1149: Logging the rule graph.

Defining log_rulegraph ensures the DAG structure is captured in logs. The function aggregates nodes and edges, returning a JSON-like structure for structured logging. Consider verifying that large DAGs won’t overwhelm logs (potential size/performance trade-off).

src/snakemake/scheduler.py (6)

20-20: Imported LogEvent for structured logging.

Consistent with other modules, this addition aids in capturing event-specific log data.


36-36: Refined error message.

Updating the text from “Look above” to “Look below” better reflects the final log ordering.


218-223: Structured logging for failed jobs.

Logging errors with extra context from each failed job is beneficial but confirm that get_log_error_info() never exposes private data.


308-312: Including structured details on job start.

Providing job IDs within LogEvent.JOB_STARTED clarifies which jobs are launching in logs. This is helpful for parallel debugging or monitoring.


407-416: Detailed finishing logs.

Appreciate the rule name and job ID in LogEvent.JOB_FINISHED. This helps quickly match up start and finish events for traceability.


841-849: Progress logging improvement.

Logging progress with LogEvent.PROGRESS allows structured updates for external consumers (e.g., dashboards).

src/snakemake/logging.py (1)

398-399: No nested if statements here.

The static analysis hint recommending a single if instead of nested ones does not seem applicable. The code at these lines does not contain nested conditionals.

🧰 Tools
🪛 Ruff (0.8.2)

398-399: Use a single if statement instead of nested if statements

Combine if statements using and

(SIM102)

docs/tutorial/setup.rst (2)

77-81: Clarity on package manager choices.

Your documentation concisely explains both Pixi and Miniforge, ensuring new users can choose the preferred option. The changes are clear and appear well-structured.


136-155: Great introduction to Pixi usage.

The newly added instructions for installing and activating Pixi provide a straightforward workflow. This helps beginners avoid confusion by having guided commands and references.

src/snakemake/dag.py (3)

2817-2843: Good addition of a structured job statistics method.

The new stats method clearly collects and presents job counts, returning both a formatted message and a dictionary for further programmatic usage. This is a neat improvement that can facilitate automation (e.g., in CI pipelines or dashboards).


397-407:

Details

❓ Verification inconclusive

Verify concurrency error handling.

The asynchronous block within lines 397-407 updates storage inputs concurrently. Be sure that partial retrieval errors are properly handled and do not leave job data in an inconsistent state.

Could you run the following script to grep for any additional concurrency checks or partial error-handling logic to ensure all tasks are reliably handled?


🏁 Script executed:

#!/bin/bash
rg -A 5 "asyncio.TaskGroup" 

Length of output: 3934


Action Required: Verify that partial storage retrieval failures do not leave inconsistent job state.

The async block at lines 397–407 in src/snakemake/dag.py uses an asyncio.TaskGroup to concurrently trigger storage retrieval via f.retrieve_from_storage(). As observed, any exceptions raised by these concurrent tasks are caught as an ExceptionGroup and rethrown as a WorkflowError. A similar pattern appears in other modules (e.g., in src/snakemake/persistence.py and src/snakemake/jobs.py), suggesting a standardized concurrency approach across the codebase.

Please confirm that each f.retrieve_from_storage() operation is designed to be atomic or idempotent. In other words, ensure that if only some of the retrieval tasks fail while others succeed, the system does not end up with a partially updated state. If there’s any risk of side effects from partial completions, consider adding a rollback or cleanup mechanism to maintain consistent job data.

🧰 Tools
🪛 Ruff (0.8.2)

402-402: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


474-477:

Details

❓ Verification inconclusive

Confirm final chunk ordering in toposorted results.

The logic for sorting pipe-grouped jobs within each topological layer is clear, but ensure large DAGs with multiple pipe groups remain consistent. Please test a large multi-pipe-group scenario to confirm stability of ordering.


🏁 Script executed:

#!/bin/bash
# Searching for test cases that might demonstrate multi-pipe-group sorting
rg -A 6 "test_pipe_group_multiple"

Length of output: 34


Verify DAG Ordering in Multi-Pipe-Group Scenarios

The ordering logic for pipe-grouped jobs appears sound, but our search for test cases (e.g. for identifiers like "test_pipe_group_multiple" or similar) produced no output. This means that there isn’t any automated verification currently in place for large DAGs with multiple pipe groups. Please manually verify the ordering with a sufficiently complex DAG and consider adding automated tests if none exist.

tests/test_github_issue_3265_respect_dryrun_delete_all/Snakefile_inner (10)

1-3: Looks good!

No issues found with setting the shell executable to bash.


4-10: LGTM for the all rule.

This rule correctly aggregates the final outputs.


11-20: Rule a is clear and concise.

Creating a dangling symlink appears intentional for testing purposes.


22-27: No shell or run block for rule b.

Relying on touch("some_dir/final") is valid for generating the file.


29-34: LGTM: Directory creation in rule c.

Implementation is straightforward with directory("empty_dir") and mkdir empty_dir.


36-41: LGTM: Directory creation in rule d.

Creates full_dir and an empty file inside. Nicely done.


43-50: Rule e is straightforward.

Touching a temp file for intermediate workflow steps works well.


52-59: LGTM for rule f.

Creates a temporary empty directory as expected.


61-68: LGTM for rule g.

Creates and populates a temporary directory with a small placeholder file.


70-75: Final aggregator rule temp looks good.

No shell needed since touch("temp_keep") suffices to mark the final file.

src/snakemake/cli.py (10)

20-21: Logger plugin registry import looks good.

This addition ensures access to external logger plugins.


1282-1290: New argument --max-checksum-file-size is a helpful addition.

Allows users to skip checksums on large files, likely improving performance. The default (1MB) seems reasonable as a starting point.


1503-1509: Custom --logger argument is nicely integrated.

Enables multiple logger plugins, promoting customizable logging strategies.


1747-1747: Registration of logger CLI args is good.

Ensures that all logger plugins can dynamically add their arguments.


1886-1890: Great approach to retrieving logger plugin settings.

The dictionary comprehension keeps code concise and scalable for multiple plugins.


1922-1922: Passing dryrun=args.dryrun is correct.

This correctly maps CLI usage for workflow output settings.


1929-1929: Binding log_handler_settings to OutputSettings is well done.

Ensures consistent usage of custom loggers within output settings.


2032-2032: Properly passing max_checksum_file_size to the DAG.

This enforces the threshold during DAG computation and job postprocessing.


2096-2096: Forwarding dryrun to delete_output is correct.

Allows users to preview which outputs would be deleted without executing changes.


76-80:

Details

✅ Verification successful

Check for humanfriendly availability.

The parse_size_in_bytes function is useful for parsing numeric sizes, but please confirm humanfriendly is installed and included in your environment or dependency metadata.


🏁 Script executed:

#!/bin/bash
# Check if humanfriendly is referenced in project dependencies
rg 'humanfriendly' -A 5

Length of output: 5169


Humanfriendly dependency verified – no additional changes needed

After verifying the codebase, it’s clear that the humanfriendly package is referenced in multiple dependency files (e.g., test-environment.yml and docs/requirements.txt) as well as throughout the code (e.g., in src/snakemake/settings/types.py, src/snakemake/resources.py, and src/snakemake/cli.py). The parse_size_in_bytes function correctly utilizes humanfriendly.parse_size, and the dependency is properly accounted for in the project.

No further modifications are required here.

@johanneskoester johanneskoester merged commit 4574c92 into main Mar 13, 2025
49 checks passed
@johanneskoester johanneskoester deleted the fixes-3271 branch March 13, 2025 16:43
johanneskoester pushed a commit that referenced this pull request Mar 14, 2025
🤖 I have created a release *beep* *boop*
---


##
[9.0.0](v8.30.0...v9.0.0)
(2025-03-14)


### ⚠ BREAKING CHANGES

* Logging refactor & add LoggerPluginInterface
([#3107](#3107))

### Features

* [#3412](#3412) - keep
shadow folder of failed job if --keep-incomplete flag is set.
([#3430](#3430))
([22978c3](22978c3))
* add flag --report-after-run to automatically generate the report after
a successfull workflow run
([#3428](#3428))
([b0a7f03](b0a7f03))
* add flatten function to IO utils
([#3424](#3424))
([67fa392](67fa392))
* add helper functions to parse input files
([#2918](#2918))
([63e45a7](63e45a7))
* Add option to print redacted file names
([#3089](#3089))
([ba4d264](ba4d264))
* add support for validation of polars dataframe and lazyframe
([#3262](#3262))
([c7473a6](c7473a6))
* added support for rendering dag with mermaid js
([#3409](#3409))
([7bf8381](7bf8381))
* adding --replace-workflow-config to fully replace workflow configs
(from config: directive) with --configfile, instead of merging them
([#3381](#3381))
([47504a0](47504a0))
* Dynamic module name
([#3401](#3401))
([024dc32](024dc32))
* Enable saving and reloading IOCache object
([#3386](#3386))
([c935953](c935953))
* files added in rule params with workflow.source_path will be available
in used containers
([#3385](#3385))
([a6e45bf](a6e45bf))
* Fix keep_local in storage directive and more freedom over remote
retrieval behaviour
([#3410](#3410))
([67b4739](67b4739))
* inherit parameters of use rule and extend/replace individual items
them when using 'with' directive
([#3365](#3365))
([93e4b92](93e4b92))
* Logging refactor & add LoggerPluginInterface
([#3107](#3107))
([86f1d6e](86f1d6e))
* Maximal file size for checksums
([#3368](#3368))
([b039f8a](b039f8a))
* Modernize package configuration using Pixi
([#3369](#3369))
([77992d8](77992d8))
* multiext support for named input/output
([#3372](#3372))
([05e1378](05e1378))
* optionally auto-group jobs via temp files in case of remote execution
([#3378](#3378))
([cc9bba2](cc9bba2))


### Bug Fixes

* `--delete-all-output` ignores `--dry-run`
([#3265](#3265))
([23fef82](23fef82))
* 3342 faster touch runs and warning messages for non-existing files
([#3398](#3398))
([cd9c3c3](cd9c3c3))
* add default value to max-jobs-per-timespan
([#3043](#3043))
([2959abe](2959abe))
* checkpoints inside modules are overwritten
([#3359](#3359))
([fba3ac7](fba3ac7))
* Convert Path to IOFile
([#3405](#3405))
([c58684c](c58684c))
* Do not perform storage object cleanup with --keep-storage-local-copies
set ([#3358](#3358))
([9a6d14b](9a6d14b))
* edgecases of source deployment in case of remote execution
([#3396](#3396))
([5da13be](5da13be))
* enhance error message formatting for strict DAG-building mode
([#3376](#3376))
([a1c39ee](a1c39ee))
* fix bug in checkpoint handling that led to exceptions in case
checkpoint output was missing upon rerun
([#3423](#3423))
([8cf4a2f](8cf4a2f))
* force check all required outputs
([#3341](#3341))
([495a4e7](495a4e7))
* group job formatting
([#3442](#3442))
([f0b10a3](f0b10a3))
* in remote jobs, upload storage in topological order such that
modification dates are preserved (e.g. in case of group jobs)
([#3377](#3377))
([eace08f](eace08f))
* only skip eval when resource depends on input
([#3374](#3374))
([4574c92](4574c92))
* Prevent execution of conda in apptainer when not explicitly requested
in software deployment method
([#3388](#3388))
([c43c5c0](c43c5c0))
* print filenames with quotes around them in RuleException
([#3269](#3269))
([6baeda5](6baeda5))
* Re-evaluation of free resources
([#3399](#3399))
([6371293](6371293))
* ReadTheDocs layout issue due to src directory change
([#3419](#3419))
([695b127](695b127))
* robustly escaping quotes in generated bash scripts (v2)
([#3297](#3297))
([#3389](#3389))
([58720bd](58720bd))
* Show apptainer image URL in snakemake report
([#3407](#3407))
([45f0450](45f0450))
* Update ReadTheDocs configuration for documentation build to use Pixi
([#3433](#3433))
([3f227a6](3f227a6))


### Documentation

* Add pixi setup instructions to general use tutorial
([#3382](#3382))
([115e81b](115e81b))
* fix contribution section heading levels, fix docs testing setup order
([#3360](#3360))
([051dc53](051dc53))
* fix link to github.com/snakemake/poetry-snakemake-plugin
([#3436](#3436))
([ec6d97c](ec6d97c))
* fix quoting
([#3394](#3394))
([b40f599](b40f599))
* fix rerun-triggers default
([#3403](#3403))
([4430e23](4430e23))
* fix typo 'safe' -&gt; 'save'
([#3384](#3384))
([7755861](7755861))
* mention code formatting in the contribution section
([#3431](#3431))
([e8682b7](e8682b7))
* remove duplicated 'functions'.
([#3356](#3356))
([7c595db](7c595db))
* update broken links documentation
([#3437](#3437))
([e3d0d88](e3d0d88))
* Updating contributing guidelines with new pixi dev setup
([#3415](#3415))
([8e95a12](8e95a12))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: snakemake-bot <snakemake-bot-admin@googlegroups.com>
kjohnsen pushed a commit to kjohnsen/snakemake that referenced this pull request Dec 15, 2025
<!--Add a description of your PR here-->

Fixes snakemake#3271.

Currently, evaluation of functions in resources are skipped until after
input is evaluated. However, this is not necessary when a function does
not rely on input. This PR amends the skipping, to only operate on
resources that depend on input.

### QC
<!-- Make sure that you can tick the boxes below. -->

* [x] The PR contains a test case for the changes or the changes are
already covered by an existing test case.
* [x] 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).


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Introduced new workflow rules that dynamically compute and display
resource allocations for memory management.
- Added a new rule to handle specific resource requirements and output
generation.

- **Tests**
- Expanded test coverage with scenarios validating both expected
successes and intentional failure cases to ensure robust workflow
behavior.
  - Introduced a new test scenario related to GitHub issue 3374.

- **Refactor**
- Streamlined internal resource evaluation logic for improved dependency
management and clarity within resource handling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Johannes Koester <johannes.koester@uni-due.de>
kjohnsen pushed a commit to kjohnsen/snakemake that referenced this pull request Dec 15, 2025
🤖 I have created a release *beep* *boop*
---


##
[9.0.0](snakemake/snakemake@v8.30.0...v9.0.0)
(2025-03-14)


### ⚠ BREAKING CHANGES

* Logging refactor & add LoggerPluginInterface
([snakemake#3107](snakemake#3107))

### Features

* [snakemake#3412](snakemake#3412) - keep
shadow folder of failed job if --keep-incomplete flag is set.
([snakemake#3430](snakemake#3430))
([22978c3](snakemake@22978c3))
* add flag --report-after-run to automatically generate the report after
a successfull workflow run
([snakemake#3428](snakemake#3428))
([b0a7f03](snakemake@b0a7f03))
* add flatten function to IO utils
([snakemake#3424](snakemake#3424))
([67fa392](snakemake@67fa392))
* add helper functions to parse input files
([snakemake#2918](snakemake#2918))
([63e45a7](snakemake@63e45a7))
* Add option to print redacted file names
([snakemake#3089](snakemake#3089))
([ba4d264](snakemake@ba4d264))
* add support for validation of polars dataframe and lazyframe
([snakemake#3262](snakemake#3262))
([c7473a6](snakemake@c7473a6))
* added support for rendering dag with mermaid js
([snakemake#3409](snakemake#3409))
([7bf8381](snakemake@7bf8381))
* adding --replace-workflow-config to fully replace workflow configs
(from config: directive) with --configfile, instead of merging them
([snakemake#3381](snakemake#3381))
([47504a0](snakemake@47504a0))
* Dynamic module name
([snakemake#3401](snakemake#3401))
([024dc32](snakemake@024dc32))
* Enable saving and reloading IOCache object
([snakemake#3386](snakemake#3386))
([c935953](snakemake@c935953))
* files added in rule params with workflow.source_path will be available
in used containers
([snakemake#3385](snakemake#3385))
([a6e45bf](snakemake@a6e45bf))
* Fix keep_local in storage directive and more freedom over remote
retrieval behaviour
([snakemake#3410](snakemake#3410))
([67b4739](snakemake@67b4739))
* inherit parameters of use rule and extend/replace individual items
them when using 'with' directive
([snakemake#3365](snakemake#3365))
([93e4b92](snakemake@93e4b92))
* Logging refactor & add LoggerPluginInterface
([snakemake#3107](snakemake#3107))
([86f1d6e](snakemake@86f1d6e))
* Maximal file size for checksums
([snakemake#3368](snakemake#3368))
([b039f8a](snakemake@b039f8a))
* Modernize package configuration using Pixi
([snakemake#3369](snakemake#3369))
([77992d8](snakemake@77992d8))
* multiext support for named input/output
([snakemake#3372](snakemake#3372))
([05e1378](snakemake@05e1378))
* optionally auto-group jobs via temp files in case of remote execution
([snakemake#3378](snakemake#3378))
([cc9bba2](snakemake@cc9bba2))


### Bug Fixes

* `--delete-all-output` ignores `--dry-run`
([snakemake#3265](snakemake#3265))
([23fef82](snakemake@23fef82))
* 3342 faster touch runs and warning messages for non-existing files
([snakemake#3398](snakemake#3398))
([cd9c3c3](snakemake@cd9c3c3))
* add default value to max-jobs-per-timespan
([snakemake#3043](snakemake#3043))
([2959abe](snakemake@2959abe))
* checkpoints inside modules are overwritten
([snakemake#3359](snakemake#3359))
([fba3ac7](snakemake@fba3ac7))
* Convert Path to IOFile
([snakemake#3405](snakemake#3405))
([c58684c](snakemake@c58684c))
* Do not perform storage object cleanup with --keep-storage-local-copies
set ([snakemake#3358](snakemake#3358))
([9a6d14b](snakemake@9a6d14b))
* edgecases of source deployment in case of remote execution
([snakemake#3396](snakemake#3396))
([5da13be](snakemake@5da13be))
* enhance error message formatting for strict DAG-building mode
([snakemake#3376](snakemake#3376))
([a1c39ee](snakemake@a1c39ee))
* fix bug in checkpoint handling that led to exceptions in case
checkpoint output was missing upon rerun
([snakemake#3423](snakemake#3423))
([8cf4a2f](snakemake@8cf4a2f))
* force check all required outputs
([snakemake#3341](snakemake#3341))
([495a4e7](snakemake@495a4e7))
* group job formatting
([snakemake#3442](snakemake#3442))
([f0b10a3](snakemake@f0b10a3))
* in remote jobs, upload storage in topological order such that
modification dates are preserved (e.g. in case of group jobs)
([snakemake#3377](snakemake#3377))
([eace08f](snakemake@eace08f))
* only skip eval when resource depends on input
([snakemake#3374](snakemake#3374))
([4574c92](snakemake@4574c92))
* Prevent execution of conda in apptainer when not explicitly requested
in software deployment method
([snakemake#3388](snakemake#3388))
([c43c5c0](snakemake@c43c5c0))
* print filenames with quotes around them in RuleException
([snakemake#3269](snakemake#3269))
([6baeda5](snakemake@6baeda5))
* Re-evaluation of free resources
([snakemake#3399](snakemake#3399))
([6371293](snakemake@6371293))
* ReadTheDocs layout issue due to src directory change
([snakemake#3419](snakemake#3419))
([695b127](snakemake@695b127))
* robustly escaping quotes in generated bash scripts (v2)
([snakemake#3297](snakemake#3297))
([snakemake#3389](snakemake#3389))
([58720bd](snakemake@58720bd))
* Show apptainer image URL in snakemake report
([snakemake#3407](snakemake#3407))
([45f0450](snakemake@45f0450))
* Update ReadTheDocs configuration for documentation build to use Pixi
([snakemake#3433](snakemake#3433))
([3f227a6](snakemake@3f227a6))


### Documentation

* Add pixi setup instructions to general use tutorial
([snakemake#3382](snakemake#3382))
([115e81b](snakemake@115e81b))
* fix contribution section heading levels, fix docs testing setup order
([snakemake#3360](snakemake#3360))
([051dc53](snakemake@051dc53))
* fix link to github.com/snakemake/poetry-snakemake-plugin
([snakemake#3436](snakemake#3436))
([ec6d97c](snakemake@ec6d97c))
* fix quoting
([snakemake#3394](snakemake#3394))
([b40f599](snakemake@b40f599))
* fix rerun-triggers default
([snakemake#3403](snakemake#3403))
([4430e23](snakemake@4430e23))
* fix typo 'safe' -&gt; 'save'
([snakemake#3384](snakemake#3384))
([7755861](snakemake@7755861))
* mention code formatting in the contribution section
([snakemake#3431](snakemake#3431))
([e8682b7](snakemake@e8682b7))
* remove duplicated 'functions'.
([snakemake#3356](snakemake#3356))
([7c595db](snakemake@7c595db))
* update broken links documentation
([snakemake#3437](snakemake#3437))
([e3d0d88](snakemake@e3d0d88))
* Updating contributing guidelines with new pixi dev setup
([snakemake#3415](snakemake#3415))
([8e95a12](snakemake@8e95a12))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: snakemake-bot <snakemake-bot-admin@googlegroups.com>
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.

accessing mem_mb from resources in params

2 participants