src/snakemake/workflow.py (1)

166-166: Validate the implications of assigning resources by reference.

self.global_resources = self.resource_settings.resources creates a direct reference rather than a copy. If unintentional shared state is not desired later, consider using a copy; otherwise, this is fine if continued synchronization is intended.

- self.global_resources = self.resource_settings.resources
+ self.global_resources = deepcopy(self.resource_settings.resources)

src/snakemake/rules.py (1)

1027-1031: Consider chaining the original exception.

According to B904, raising a new exception in an except block can be improved by chaining the old exception (raise ... from err). If the original context is unnecessary, you could also use from None.

-except ResourceValidationError:
-    raise WorkflowError(
+except ResourceValidationError as err:
+    raise WorkflowError(

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

src/snakemake/cli.py (1)

183-202: Consider removing obsolete code.

The commented-out code appears fully replaced by the new resource parsing logic. To keep the codebase clean, consider removing it rather than commenting it out.

-    # (Old commented-out code here)
-    # ...
+    # Remove this entire commented section to avoid confusion

tests/common.py (1)

442-446: Clarity in test failure logic.

The new checks handle boolean vs. exception usage correctly. The nested condition can be slightly confusing but remains logically sound.

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

353-354: Default resource mode.

Setting Resources.default("bare") is likely correct for minimal resource usage. Ensure it’s intentional versus "full" elsewhere for consistency.

src/snakemake/jobs.py (1)

122-132: Use the local variable v for consistency.
Within the dictionary comprehension, you already unpack (k, v). For clarity and readability, prefer referencing v instead of self.resources[k].

-        res_dict = {
-            k: v
-            for k, v in self.resources.items()
-            if not self.dag.workflow.resource_scopes.is_local(k)
-            and not isinstance(self.resources[k], TBDString)
-        }
+        res_dict = {
+            k: v
+            for k, v in self.resources.items()
+            if not self.dag.workflow.resource_scopes.is_local(k)
+            and not isinstance(v, TBDString)
+        }

tests/tests.py (4)

800-801: Clarify removed assertion snippet.

The commented-out assertion in line 801 might still be relevant or helpful for debugging. Either remove it entirely or convert it into an explanatory comment if it contains important context:

-# assert {res: resources[res] for res in expected_resources} == expected_resources
+# If needed in debugging, consider providing some clarifying context here

---

1054-1058: Eliminate ambiguous variable name and optimize comparison.

Instead of l and not l == "\n", rename the loop variable to something descriptive and use != for clarity. Example fix:

- lines = [l for l in f.readlines() if not l == "\n"]
+ lines = [line_content for line_content in f.readlines() if line_content != "\n"]

🧰 Tools

🪛 Ruff (0.8.2)

1054-1054: Ambiguous variable name: l

(E741)

---

1054-1054: Use l != "\n" instead of not l == "\n"

Replace with != operator

(SIM201)

---

1131-1131: Remove or refine explanatory comment.

"Implicitly tests the normalization of mem_mb and disk_mb..." can be clarified further or removed if self-explanatory. Comment clarity aids future maintainers.

---

1148-1148: Consider verifying all suffixed forms.

You assert combined resource fields (mem=60000, disk=3000). Adding checks for _mib or subtle edge cases would ensure that normalizations remain robust across future code changes.

src/snakemake/resources.py (12)

2-29: Remove unused imports.

Imports such as TypeVar, UserDict, Dict, Generic, Tuple, Union, isin appear unused and can be removed to keep the code clean.

Apply an example diff:

- from ast import TypeVar
- from collections import UserDict, defaultdict
+ from collections import defaultdict
...
- from typing import TYPE_CHECKING, Any, Callable, Dict, ...
+ from typing import TYPE_CHECKING, Any, Callable, ...
...
- from numpy import isin

🧰 Tools

🪛 Ruff (0.8.2)

2-2: ast.TypeVar imported but unused

Remove unused import: ast.TypeVar

(F401)

---

3-3: collections.UserDict imported but unused

Remove unused import: collections.UserDict

(F401)

---

16-16: typing.Dict imported but unused

Remove unused import

(F401)

---

17-17: typing.Generic imported but unused

Remove unused import

(F401)

---

23-23: typing.Tuple imported but unused

Remove unused import

(F401)

---

25-25: typing.Union imported but unused

Remove unused import

(F401)

---

29-29: numpy.isin imported but unused

Remove unused import: numpy.isin

(F401)

---

135-137: Proper initialization of total_resources and blocks.

Using defaultdict(int) for total_resources plus an ordinary list for blocks is consistent. Confirm that these data structures are reset for each new group job evaluation to avoid stale state if the method is reused.

---

141-144: Check for empty sibling blocks.

block_resources, job_resources, and pipe_resources are all set up well. Provide test coverage for empty sibling sets or no pipe groups to confirm stable handling of degenerate cases.

---

190-190: Centralize local variable naming.

resource_types = list(set(it.chain(*job_resources))) is clear though a bit nested. Consider a short comment clarifying it merges all resource keys from multiple jobs for readability.

---

219-219: Validate sortby usage.

Bringing in sortby from the constraints ensures layered scheduling. Consider logging or debugging prints if resource layering leads to unexpected group job expansions.

---

386-390: Clarify _proportion and _highest_proportion docstrings.

These helper functions are crucial for layering logic. Add docstrings or comments to highlight that 0 is returned if there is no constraint.

---

394-394: Initializing rows.

rows: list[list[tuple[int, ...]]] = [[]] works for the layering approach. Include a quick comment clarifying why you start with a single empty row rather than an empty list for better maintainability.

---

431-433: Warn on single-resource overshoot.

Exceeding constraints in pipe jobs is flagged here. The current messaging is succinct. If possible, mention the job or pipe group name for easier debugging.

---

596-596: Check type comparisons.

Instead of if type(self_val) != type(other_val):, prefer if not isinstance(self_val, type(other_val)): for clarity and best practice:

-if type(self_val) != type(other_val):
+if not isinstance(self_val, type(other_val)):

🧰 Tools

🪛 Ruff (0.8.2)

596-596: Use is and is not for type comparisons, or isinstance() for isinstance checks

(E721)

---

754-765: Raise exceptions with explicit cause.

Pythonic style recommends chaining exceptions for clarity:

-except Exception as e:
-    raise WorkflowError(...) 
+except Exception as e:
+    raise WorkflowError(...) from e

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

805-805: Use 'is None' checks.

Line 805 compares if mapping == None:; prefer if mapping is None: to comply with Python style.

🧰 Tools

🪛 Ruff (0.8.2)

805-805: Comparison to None should be cond is None

Replace with cond is None

(E711)

---

856-856: Consider using exception chaining.

When re-raising a caught exception, do:

raise ValueError("...") from err

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

src/snakemake/resources.py (1)

293-299: 🛠️ Refactor suggestion

Avoid mutable defaults in function parameters.

skip: list[str] = [] and methods: dict[str, Callable[[list[int]], int]] = {} can introduce subtle bugs. Use None and initialize inside the function:

-def _merge_resource_dict(
-    cls,
-    resources: list[dict[str, str | int]],
-    skip: list[str] = [],
-    methods: dict[str, Callable[[list[int]], int]] = {},
-    default_method: Callable[[list[int]], int] = max,
-):
+def _merge_resource_dict(
+    cls,
+    resources: list[dict[str, str | int]],
+    skip: Optional[list[str]] = None,
+    methods: Optional[dict[str, Callable[[list[int]], int]]] = None,
+    default_method: Callable[[list[int]], int] = max,
+):
+    if skip is None:
+        skip = []
+    if methods is None:
+        methods = {}

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

    def _merge_resource_dict(
        cls,
        resources: list[dict[str, str | int]],
        skip: Optional[list[str]] = None,
        methods: Optional[dict[str, Callable[[list[int]], int]]] = None,
        default_method: Callable[[list[int]], int] = max,
    ):
        if skip is None:
            skip = []
        if methods is None:
            methods = {}

🧰 Tools

🪛 Ruff (0.8.2)

296-296: Do not use mutable data structures for argument defaults

Replace with None; initialize within function

(B006)

---

297-297: Do not use mutable data structures for argument defaults

Replace with None; initialize within function

(B006)

src/snakemake/workflow.py (2)

1764-1777: Enhanced threads handling with proper error handling.

The try-except block catches ResourceValidationError with a clear error message. The conditional logic for thread assignment is well-structured and clear.

However, there's a potential improvement for the exception handling:

-            except ResourceValidationError:
-                raise RuleException(
+            except ResourceValidationError as err:
+                raise RuleException(
                    "Threads value has to be an integer, float, or a callable.",
                    rule=rule,
-                )
+                ) from err

Adding from err to the exception would preserve the exception chain for better debugging.

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

1778-1791: Improved resource validation.

The updated resource handling code properly validates resources and provides clear error messages. The error message includes the type of the problematic resource.

Similarly to the previous comment, consider preserving the exception chain:

-                    raise RuleException(
+                    raise RuleException(
                        "Resources values have to be integers, strings, or callables "
                        f"(functions): type({err.args[0]} : {err.args[1]}) == "
                        f"{type(err.args[1])}",
                        rule=rule,
-                    )
+                    ) from err

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

src/snakemake/rules.py (2)

1031-1058: Improved resource evaluation with clear error handling.

The new evaluate function is a significant improvement:

1. It properly handles resource evaluation with optional skipping
2. Provides clear error messages for validation failures
3. Uses the new resource types consistently

The error handling is comprehensive, capturing both ResourceValidationError and general exceptions.

Consider preserving the exception chain for clearer debugging in these exception handlers:

-            except ResourceValidationError:
-                raise WorkflowError(
+            except ResourceValidationError as err:
+                raise WorkflowError(
                    f"Resource {resource} is neither int, float (would be rounded to "
                    "nearest int), str, or None.",
                    rule=self,
-                )
+                ) from err

And similarly for the BaseException handler.

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

1068-1094: Robust resource evaluation and constraint checking.

This section properly evaluates resources, handles None values, and applies constraints from global resources with appropriate type checking. The error message for type mismatches is clear and informative.

Again, consider preserving the exception chain:

-            except ResourceConstraintError:
+            except ResourceConstraintError as err:
                global_val = global_res.value
                global_type = (
                    "an int" if isinstance(global_val, int) else type(global_val)
                )
-                raise WorkflowError(
+                raise WorkflowError(
                    f"Resource {resource} is of type {type(val.value).__name__} but "
                    f"global resource constraint defines {global_type} with value "
                    f"{global_val}. Resources with the same name need to have the same "
                    "types (int, float, or str are allowed).",
                    rule=self,
-                )
+                ) from err

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

tests/tests.py (1)

1136-1136: Fix spelling error in comment

There's a typo in the comment: "Implicitely" should be "Implicitly".

-    # Implicitely tests the normalization of mem_mb and disk_mb to mem and disk
+    # Implicitly tests the normalization of mem_mb and disk_mb to mem and disk

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 1136-1136:
Implicitely ==> Implicitly

🪛 GitHub Actions: Codespell

[error] 1136-1136: codespell: Implicitely ==> Implicitly

src/snakemake/resources.py (7)

1-29: Clean up unused imports

Several imports are declared but never used in the code, which can lead to maintainability issues.

from __future__ import annotations
-from ast import TypeVar
from collections import UserDict, defaultdict
import copy
from dataclasses import dataclass
from humanfriendly import InvalidTimespan, InvalidSize, parse_size, parse_timespan
import itertools as it
import operator as op
import re
import tempfile
import math
from typing import (
    TYPE_CHECKING,
    Any,
    Callable,
-    Dict,
-    Generic,
    Iterator,
    Literal,
    Mapping,
    Self,
    Sequence,
-    Tuple,
    TypeAlias,
-    Union,
    cast,
)

-from numpy import isin

🧰 Tools

🪛 Ruff (0.8.2)

2-2: ast.TypeVar imported but unused

Remove unused import: ast.TypeVar

(F401)

---

3-3: collections.UserDict imported but unused

Remove unused import: collections.UserDict

(F401)

---

16-16: typing.Dict imported but unused

Remove unused import

(F401)

---

17-17: typing.Generic imported but unused

Remove unused import

(F401)

---

23-23: typing.Tuple imported but unused

Remove unused import

(F401)

---

25-25: typing.Union imported but unused

Remove unused import

(F401)

---

29-29: numpy.isin imported but unused

Remove unused import: numpy.isin

(F401)

---

525-538: Replace if-else with ternary in repr method

A small optimization opportunity in the repr method.

def __repr__(self):
-    if self.is_evaluable():
-        value = "function(...)"
-    else:
-        value = self._value
+    value = "function(...)" if self.is_evaluable() else self._value
    return f'Resource("{self.name}", {value})'

🧰 Tools

🪛 Ruff (0.8.2)

526-529: Use ternary operator value = "function(...)" if self.is_evaluable() else self._value instead of if-else-block

Replace if-else-block with value = "function(...)" if self.is_evaluable() else self._value

(SIM108)

---

566-577: Fix typo in docstring

There's a typo in the docstring for the evaluate method.

def evaluate(self, *args: Any, **kwargs: Any):
    """Evaluate the resource with given args.

    This method ensures the returned resource is correctly returned in a
-    new ``Resource`` class, validated, and standardized. Resources should thus
-    only be evaulated via this method.
+    new ``Resource`` class, validated, and standardized. Resources should thus
+    only be evaluated via this method.
    """

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 571-571:
evaulated ==> evaluated

---

674-720: Fix typo in docstring

There's a typo in the docstring for the from_cli_expression method.

@classmethod
def from_cli_expression(
    cls, name: str, value: str, *, with_threads_arg: bool = True
):
    """Create a new evaluable resource based on a python expression.

    Threads can optionally be included in the expression environment

    Arguments
    =========
    name:
        name of the resource
    value:
-        python expression to be evaulated
+        python expression to be evaluated
    with_threads_arg: boolean
        If True, include ``threads`` as an argument in the returned function
    """

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 687-687:
evaulated ==> evaluated

---

898-965: Fix typo in docstring

There's a typo in the docstring for the parse method.

@classmethod
def parse(
    cls,
    exprs: list[str],
    *,
    allow_expressions: bool = False,
    only_positive_integers: bool = False,
    defaults: None | Literal["bare"] | Literal["full"] = None,
):
    """Parse a series of CLI-style string resource assigments.
-    """
+    """Parse a series of CLI-style string resource assignments.
+    """

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 907-907:
assigments ==> assignments

---

487-487: Fix typo in docstring

There's a typo in the docstring for the Resource class.

"""Standardized representation of a resource name and value.

In addition to ``int`` and ``str``, the standard resource types, ``Resource`` can
store ``None``, signifying an unset resource, and a callable, signifying a resource
to be calculated in the context of a rule. Via the ``.value`` property, ``Resource``
-ensures a resource has been computed before its value is retreived.
+ensures a resource has been computed before its value is retrieved.
"""

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 487-487:
retreived ==> retrieved

---

492-492: Fix typo in docstring

There's a typo in the docstring for the Resource class.

The class additionally handles input validation and standardization.
``float``s get rounded into ``int``. If the resource name corresponds to a size
resource ("disk" and "mem") or a timespan resource ("runtime"), strings values
-will be intepreted as a human readable resource and converted to an integer of
+will be interpreted as a human readable resource and converted to an integer of
appropriate denomination (Mb for size, hr for timespan).

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 492-492:
intepreted ==> interpreted

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

src/snakemake/workflow.py (2)

1767-1780: Improved thread resource handling

The thread resource handling has been refactored with a try-except block for ResourceValidationError, providing better error messages when thread values are invalid.

However, there's a minor issue with the error handling:

-            except ResourceValidationError:
-                raise RuleException(
-                    "Threads value has to be an integer, float, or a callable.",
-                    rule=rule,
-                )
+            except ResourceValidationError as err:
+                raise RuleException(
+                    "Threads value has to be an integer, float, or a callable.",
+                    rule=rule,
+                ) from err

Adding from err preserves the exception chain, which helps with debugging by showing the original cause.

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

1781-1795: Improved resource validation and handling

Added proper error handling for resource parsing with informative error messages. Now resources are handled through the Resources class API with Resources.from_mapping() rather than direct dictionary operations.

The error handling could be improved by preserving the exception chain:

-                except KeyError as err:
-                    raise RuleException(
-                        "Resources values have to be integers, strings, or callables "
-                        f"(functions): type({err.args[0]} : {err.args[1]}) == "
-                        f"{type(err.args[1])}",
-                        rule=rule,
-                    )
+                except KeyError as err:
+                    raise RuleException(
+                        "Resources values have to be integers, strings, or callables "
+                        f"(functions): type({err.args[0]} : {err.args[1]}) == "
+                        f"{type(err.args[1])}",
+                        rule=rule,
+                    ) from err

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

src/snakemake/rules.py (1)

1068-1094: Enhanced resource evaluation and constraint application

Improved the handling of all resources with proper evaluation and constraint checking:

1. Skips _cores which is handled separately
2. Only evaluates resources that have evaluation functions
3. Properly handles None values
4. Applies constraints against global resources with proper error messages
5. Includes type checking to ensure resources are of compatible types

This makes resource handling more robust and consistent.

The exception handling at line 1086 could benefit from using from err to preserve the exception chain:

-                raise WorkflowError(
-                    f"Resource {resource} is of type {type(val.value).__name__} but "
-                    f"global resource constraint defines {global_type} with value "
-                    f"{global_val}. Resources with the same name need to have the same "
-                    "types (int, float, or str are allowed).",
-                    rule=self,
-                )
+                raise WorkflowError(
+                    f"Resource {resource} is of type {type(val.value).__name__} but "
+                    f"global resource constraint defines {global_type} with value "
+                    f"{global_val}. Resources with the same name need to have the same "
+                    "types (int, float, or str are allowed).",
+                    rule=self,
+                ) from err

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

tests/tests.py (1)

1093-1093: Consider using != operator and improving variable name clarity.

The line uses a negated equality check (not l == "\n") which could be more clearly written as l != "\n". Additionally, the variable name l is ambiguous and could be renamed to something more descriptive like line.

- lines = [l for l in f.readlines() if not l == "\n"]
+ lines = [line for line in f.readlines() if line != "\n"]

🧰 Tools

🪛 Ruff (0.8.2)

1093-1093: Ambiguous variable name: l

(E741)

---

1093-1093: Use l != "\n" instead of not l == "\n"

Replace with != operator

(SIM201)

src/snakemake/resources.py (8)

219-220: Use from err when re-raising exceptions for better traceability

When re-raising an exception from a caught exception, use raise ... from err to preserve the original exception's traceback, which helps with debugging.

            except ResourceError as err:
-                raise ResourceInsufficiencyError(additive_resources, err.args[0])
+                raise ResourceInsufficiencyError(additive_resources, err.args[0]) from err

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

508-514: Consider using a ternary operator for better readability

You can simplify the if-else block with a ternary operator for a more concise representation.

    def __repr__(self):
-        if self.is_evaluable():
-            value = "function(...)"
-        else:
-            value = self._value
+        value = "function(...)" if self.is_evaluable() else self._value
        return f'Resource("{self.name}", {value})'

🧰 Tools

🪛 Ruff (0.8.2)

509-512: Use ternary operator value = "function(...)" if self.is_evaluable() else self._value instead of if-else-block

Replace if-else-block with value = "function(...)" if self.is_evaluable() else self._value

(SIM108)

---

578-580: Use isinstance() instead of type comparison

Using isinstance() is preferred over direct type comparison as it handles inheritance properly. The current implementation could fail if other_val is a subclass of the type of self_val.

        self_val = self.value
-        if not isinstance(self_val, type(other_val)):
+        if not isinstance(self_val, type(other_val)) or not isinstance(other_val, type(self_val)):
            raise ResourceConstraintError(self_val, other_val)

---

586-638: Consider reducing code duplication between to_mib and to_mb methods

The to_mib and to_mb methods share very similar structure and logic. Consider refactoring to reduce duplication by creating a helper method that handles the common logic.

You could create a private helper method like _convert_units that both methods use:

def _convert_units(self, target_suffix, conversion_func, evaluable_wrapper):
    if isinstance(self._value, TBDString):
        value = self._value
    elif self.is_evaluable():
        value = evaluable_wrapper(self.name, self._get_evaluator())
    elif not isinstance(self._value, int):
        errmsg = (
            f"Resource must be of type 'int' to convert to {target_suffix}. {self.name} == "
            f"{self._value} (type {type(self._value)})"
        )
        raise TypeError(errmsg)
    else:
        value = conversion_func(self._value)
    return Resource(f"{self.name.removesuffix(target_suffix == 'mib' and '_mb' or '_mib')}_{target_suffix}", value)

def to_mib(self):
    """Convert the resource to mebibytes..."""
    return self._convert_units('mib', mb_to_mib, evaluable_from_mb_to_mib)

def to_mb(self):
    """Convert the resource to megabytes..."""
    return self._convert_units('mb', mib_to_mb, evaluable_from_mib_to_mb)

---

735-748: Use from err or from None when re-raising exceptions

When re-raising an exception from a caught exception, use raise ... from err to preserve the original exception's traceback, or from None to suppress it intentionally.

            raise WorkflowError(
                "Failed to evaluate resources value "
                f"'{val}'.\n"
                "    String arguments may need additional "
                "quoting. E.g.: --default-resources "
                "\"tmpdir='/home/user/tmp'\" or "
                "--set-resources \"somerule:someresource='--nice=100'\". "
                "This also holds for setting resources inside of a profile, where "
                "you might have to enclose them in single and double quotes, "
                "i.e. someresource: \"'--nice=100'\".",
-                e,
+                e, 
+            ) from e

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

835-839: Use from err when re-raising exceptions

When re-raising an exception from a caught exception, include from err to preserve the traceback for better debugging.

    def decode_arg(cls, arg: str) -> Tuple[str, str]:
        try:
            return cast(Tuple[str, str], tuple(arg.split("=", maxsplit=1)))
        except ValueError:
-            raise ValueError("Resources have to be defined as name=value pairs.")
+            raise ValueError("Resources have to be defined as name=value pairs.") from None

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

845-854: Use from err when re-raising exceptions

Include from None when re-raising an exception to clarify that this is a new exception with no traceback linkage to a previous one.

        try:
            default_resources = cls.DEFAULTS[mode or "full"]
        except KeyError:
-            raise ValueError(f"Unexpected mode for DefaultResources: {mode}")
+            raise ValueError(f"Unexpected mode for DefaultResources: {mode}") from None

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

778-786: Consider moving defaults to a configuration file

Hardcoded default configurations can be difficult to maintain and change. Consider moving these defaults to a configuration file that can be loaded at runtime.

This would make it easier to update default resources without modifying code, and potentially allow users to customize defaults without code changes.

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

src/snakemake/resources.py (3)

273-282: 🛠️ Refactor suggestion

Fix mutable default arguments

Using mutable default arguments (empty lists or dictionaries) can lead to unexpected behavior when the function is called multiple times.

def _merge_resource_dict(
    cls,
    resources: List[Dict[str, str | int]],
-    skip: Optional[List[str]] = None,
-    methods: Optional[Dict[str, Callable[[List[int]], int]] = None,
+    skip: Optional[List[str]] = None,
+    methods: Optional[Dict[str, Callable[[List[int]], int]] = None,
    default_method: Callable[[List[int]], int] = max,
):
    skip = skip or []
    methods = methods or {}

---

589-595: 🛠️ Refactor suggestion

Use isinstance() instead of type comparison

The constrain method uses direct type comparison, which should be replaced with isinstance().

def constrain(self, other: Resource | int | None):
    """Use ``other`` as the maximum value for ``Self``, but only if both are integers.

    Returns self unaltered if either resource is ``None`` or if both are the same
    non-integer type.

    Errors if ``other`` has a different type from ``Self``, or if either are
    evaluable resources.
    """
    if isinstance(self._value, TBDString):
        return self

    other_val = other.value if isinstance(other, Resource) else other

    if self._value is None or other_val is None:
        return self

    self_val = self.value
-    if not isinstance(self_val, type(other_val)):
+    if not isinstance(self_val, type(other_val)) or not isinstance(other_val, type(self_val)):
        raise ResourceConstraintError(self_val, other_val)
    if isinstance(self_val, int):
        assert isinstance(other_val, int)
        return Resource(self.name, min(other_val, self_val))
    return self

---

799-802: 🛠️ Refactor suggestion

Use "is None" instead of "== None"

Using is None is the proper way to check for None in Python.

def __init__(self, mapping: Dict[str, Resource] | None = None):
-    if mapping == None:
+    if mapping is None:
        self._data: Dict[str, Resource] = {}
        return
    self._data = mapping

src/snakemake/resources.py (8)

29-29: Remove unused imports

The get_function_params and overwrite_function_params are imported but not used in this file.

-from snakemake.common import get_function_params, get_input_function_aux_params, mb_to_mib, mib_to_mb, overwrite_function_params
+from snakemake.common import get_input_function_aux_params, mb_to_mib, mib_to_mb

🧰 Tools

🪛 Ruff (0.8.2)

29-29: snakemake.common.get_function_params imported but unused

Remove unused import

(F401)

---

29-29: snakemake.common.overwrite_function_params imported but unused

Remove unused import

(F401)

---

477-477: Fix typo in docstring

The word "retreived" is misspelled and should be "retrieved".

-    ensures a resource has been computed before its value is retreived.
+    ensures a resource has been computed before its value is retrieved.

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 477-477:
retreived ==> retrieved

🪛 GitHub Actions: Codespell

[error] 477-477: codespell: 'retreived' should be 'retrieved'.

---

482-482: Fix typo in docstring

The word "intepreted" is misspelled and should be "interpreted".

-    will be intepreted as a human readable resource and converted to an integer of
+    will be interpreted as a human readable resource and converted to an integer of

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 482-482:
intepreted ==> interpreted

---

564-564: Fix typo in docstring

The word "evaulated" is misspelled and should be "evaluated".

-    standardized. Resources should thus only be evaulated via this method.
+    standardized. Resources should thus only be evaluated via this method.

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 564-564:
evaulated ==> evaluated

---

680-680: Fix typo in docstring

The word "evaulated" is misspelled and should be "evaluated".

-    value: python expression to be evaulated
+    value: python expression to be evaluated

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 680-680:
evaulated ==> evaluated

---

901-903: Fix typo in docstring

The word "assigments" is misspelled and should be "assignments".

-    """Parse a series of CLI-style string resource assigments.
+    """Parse a series of CLI-style string resource assignments.

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 901-901:
assigments ==> assignments

---

220-220: Improve exception chain handling

When raising an exception in an except block, you should use raise ... from err to preserve the exception chain for better debugging.

        try:
            layers = cls._get_layers(
                int_resources, list(sorted_constraints.values()), sortby
            )
        except ResourceError as err:
-            raise ResourceInsufficiencyError(additive_resources, err.args[0])
+            raise ResourceInsufficiencyError(additive_resources, err.args[0]) from err

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

529-533: Use ternary operator for conditional assignment

You can simplify this conditional assignment using a Python ternary operator.

def __repr__(self):
-    if self.is_evaluable():
-        value = "function(...)"
-    else:
-        value = self._value
+    value = "function(...)" if self.is_evaluable() else self._value
    return f'Resource("{self.name}", {value})'

🧰 Tools

🪛 Ruff (0.8.2)

529-532: Use ternary operator value = "function(...)" if self.is_evaluable() else self._value instead of if-else-block

Replace if-else-block with value = "function(...)" if self.is_evaluable() else self._value

(SIM108)

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

src/snakemake/resources.py (2)

589-594: 🛠️ Refactor suggestion

Use isinstance() instead of type comparison.

Direct type comparison should be replaced with isinstance() for more robust type checking.

-        if not isinstance(self_val, type(other_val)):
+        if not isinstance(self_val, type(other_val)) or not isinstance(other_val, type(self_val)):
            raise ResourceConstraintError(self_val, other_val)

---

275-282: 🛠️ Refactor suggestion

Fix mutable default arguments.

Using mutable default arguments (empty lists or dictionaries) can lead to unexpected behavior when the function is called multiple times.

    @classmethod
    def _merge_resource_dict(
        cls,
        resources: List[Dict[str, str | int]],
-        skip: Optional[List[str]] = None,
-        methods: Optional[Dict[str, Callable[[List[int]], int]]] = None,
+        skip: Optional[List[str]] = None,
+        methods: Optional[Dict[str, Callable[[List[int]], int]]] = None,
        default_method: Callable[[List[int]], int] = max,
    ):
        skip = skip or []
        methods = methods or {}

src/snakemake/resources.py (11)

29-29: Remove unused imports.

The imports get_function_params and overwrite_function_params are not used in this file.

-from snakemake.common import get_function_params, get_input_function_aux_params, mb_to_mib, mib_to_mb, overwrite_function_params
+from snakemake.common import get_input_function_aux_params, mb_to_mib, mib_to_mb

🧰 Tools

🪛 Ruff (0.8.2)

29-29: snakemake.common.get_function_params imported but unused

Remove unused import

(F401)

---

29-29: snakemake.common.overwrite_function_params imported but unused

Remove unused import

(F401)

---

471-477: Good class documentation with spelling error.

The class documentation clearly explains the purpose and behavior of the Resource class, but contains a spelling error in line 477.

-    ensures a resource has been computed before its value is retreived.
+    ensures a resource has been computed before its value is retrieved.

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 477-477:
retreived ==> retrieved

🪛 GitHub Actions: Codespell

[error] 477-477: codespell: 'retreived' is a misspelling. Correct spelling is 'retrieved'.

---

480-484: Fix spelling error in documentation.

There's a spelling error in the documentation explaining the class behavior.

-    will be intepreted as a human readable resource and converted to an integer of
+    will be interpreted as a human readable resource and converted to an integer of

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 482-482:
intepreted ==> interpreted

---

529-534: Simplify code with ternary operator.

This if-else block can be simplified using a ternary operator.

-    def __repr__(self):
-        if self.is_evaluable():
-            value = "function(...)"
-        else:
-            value = self._value
-        return f'Resource("{self.name}", {value})'
+    def __repr__(self):
+        value = "function(...)" if self.is_evaluable() else self._value
+        return f'Resource("{self.name}", {value})'

🧰 Tools

🪛 Ruff (0.8.2)

529-532: Use ternary operator value = "function(...)" if self.is_evaluable() else self._value instead of if-else-block

Replace if-else-block with value = "function(...)" if self.is_evaluable() else self._value

(SIM108)

---

564-564: Fix spelling error in documentation.

There's a spelling error in the documentation for the evaluate method.

-        This method ensures the returned resource is correctly returned in a
-        new ``Resource`` class, validated, and standardized. Resources should thus
-        only be evaulated via this method.
+        This method ensures the returned resource is correctly returned in a
+        new ``Resource`` class, validated, and standardized. Resources should thus
+        only be evaluated via this method.

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 564-564:
evaulated ==> evaluated

---

680-680: Fix spelling error in documentation.

There's a spelling error in the documentation for the from_cli_expression method.

-        value:
-            python expression to be evaulated
+        value:
+            python expression to be evaluated

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 680-680:
evaulated ==> evaluated

---

748-759: Add exception chaining with 'from' clause.

When raising a new exception from a caught exception, use the from clause to preserve the traceback.

-            raise WorkflowError(
+            raise WorkflowError(
                "Failed to evaluate resources value "
                f"'{val}'.\n"
                "    String arguments may need additional "
                "quoting. E.g.: --default-resources "
                "\"tmpdir='/home/user/tmp'\" or "
                "--set-resources \"somerule:someresource='--nice=100'\". "
                "This also holds for setting resources inside of a profile, where "
                "you might have to enclose them in single and double quotes, "
                "i.e. someresource: \"'--nice=100'\".",
-                e,
+                e,
+            ) from e

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

900-902: Fix spelling error in documentation.

There's a spelling error in the documentation for the parse method.

-        """Parse a series of CLI-style string resource assigments.
+        """Parse a series of CLI-style string resource assignments.

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 901-901:
assigments ==> assignments

---

220-220: Add exception chaining with 'from' clause.

When raising a new exception from a caught exception, use the from clause to preserve the traceback.

-            except ResourceError as err:
-                raise ResourceInsufficiencyError(additive_resources, err.args[0])
+            except ResourceError as err:
+                raise ResourceInsufficiencyError(additive_resources, err.args[0]) from err

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

850-850: Add exception chaining with 'from' clause.

When raising a new exception from a caught exception, use the from clause to preserve the traceback.

-        except ValueError:
-            raise ValueError("Resources have to be defined as name=value pairs.")
+        except ValueError as e:
+            raise ValueError("Resources have to be defined as name=value pairs.") from e

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

864-864: Add exception chaining with 'from' clause.

When raising a new exception from a caught exception, use the from clause to preserve the traceback.

-        except KeyError:
-            raise ValueError(f"Unexpected mode for DefaultResources: {mode}")
+        except KeyError as e:
+            raise ValueError(f"Unexpected mode for DefaultResources: {mode}") from e

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

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

src/snakemake/io.py (1)

1636-1638: Consider converting the standalone string literal to a comment or docstring.
If the string on line 1637 is intended as documentation (rather than just an unused literal), consider using a code comment or adding it to a docstring block for clarity.

+# This type variable is for self-returning methods on Namedlist-deriving classes.

tests/tests.py (1)

835-836: Optional debug line can be removed or documented.
Line 836 is commented out. If it is no longer needed, you may remove it to keep the codebase clean, or add a short explanation.

-    default_resources=(["mem_mb=0"]),
-    # cleanup=False
+    default_resources=(["mem_mb=0"]),

src/snakemake/resources.py (12)

29-29: Remove unused imports.

The imports get_function_params and overwrite_function_params appear to be unused in this file.

-from snakemake.common import get_function_params, get_input_function_aux_params, mb_to_mib, mib_to_mb, overwrite_function_params
+from snakemake.common import get_input_function_aux_params, mb_to_mib, mib_to_mb

🧰 Tools

🪛 Ruff (0.8.2)

29-29: snakemake.common.get_function_params imported but unused

Remove unused import

(F401)

---

29-29: snakemake.common.overwrite_function_params imported but unused

Remove unused import

(F401)

---

220-220: Use explicit exception chaining.

When raising exceptions inside an except block, you should use raise ... from err to maintain the exception chain for better debugging.

-            raise ResourceInsufficiencyError(additive_resources, err.args[0])
+            raise ResourceInsufficiencyError(additive_resources, err.args[0]) from err

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

479-479: Fix typo in docstring.

There's a spelling error in the documentation.

-    ensures a resource has been computed before its value is retreived.
+    ensures a resource has been computed before its value is retrieved.

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 479-479:
retreived ==> retrieved

🪛 GitHub Actions: Codespell

[error] 479-479: codespell: 'retreived' is a misspelling. Suggested correction: 'retrieved'.

---

484-484: Fix typo in docstring.

There's a spelling error in the documentation.

-    will be intepreted as a human readable resource and converted to an integer of
+    will be interpreted as a human readable resource and converted to an integer of

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 484-484:
intepreted ==> interpreted

---

531-534: Use ternary operator for conciseness.

Consider using a ternary operator to improve code readability and conciseness.

-        if self.is_evaluable():
-            value = "function(...)"
-        else:
-            value = self._value
+        value = "function(...)" if self.is_evaluable() else self._value

🧰 Tools

🪛 Ruff (0.8.2)

531-534: Use ternary operator value = "function(...)" if self.is_evaluable() else self._value instead of if-else-block

Replace if-else-block with value = "function(...)" if self.is_evaluable() else self._value

(SIM108)

---

566-566: Fix typo in docstring.

There's a spelling error in the documentation.

-        This method ensures the returned resource is correctly returned in a
-        new ``Resource`` class, validated, and standardized. Resources should thus
-        only be evaulated via this method.
+        This method ensures the returned resource is correctly returned in a
+        new ``Resource`` class, validated, and standardized. Resources should thus
+        only be evaluated via this method.

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 566-566:
evaulated ==> evaluated

---

590-596: Improve type comparison logic.

The current type comparison isn't symmetric. Consider using a more clear approach to check if types match.

-        if not isinstance(self_val, type(other_val)):
+        if type(self_val) != type(other_val):
            raise ResourceConstraintError(self_val, other_val)
        if isinstance(self_val, int):
            assert isinstance(other_val, int)
            return Resource(self.name, min(other_val, self_val))
        return self

---

682-682: Fix typo in docstring.

There's a spelling error in the documentation.

-            python expression to be evaulated
+            python expression to be evaluated

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 682-682:
evaulated ==> evaluated

---

750-761: Use explicit exception chaining.

When raising exceptions inside an except block, you should use raise ... from e to maintain the exception chain for better debugging.

             raise WorkflowError(
                 "Failed to evaluate resources value "
                 f"'{val}'.\n"
                 "    String arguments may need additional "
                 "quoting. E.g.: --default-resources "
                 "\"tmpdir='/home/user/tmp'\" or "
                 "--set-resources \"somerule:someresource='--nice=100'\". "
                 "This also holds for setting resources inside of a profile, where "
                 "you might have to enclose them in single and double quotes, "
                 "i.e. someresource: \"'--nice=100'\".",
-                e,
+                e) from e

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

852-852: Use explicit exception chaining.

When raising exceptions inside an except block, you should use raise ... from None to explicitly suppress the original exception while maintaining clarity.

-            raise ValueError("Resources have to be defined as name=value pairs.")
+            raise ValueError("Resources have to be defined as name=value pairs.") from None

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

866-866: Use explicit exception chaining.

When raising exceptions inside an except block, you should use raise ... from None to explicitly suppress the original exception while maintaining clarity.

-            raise ValueError(f"Unexpected mode for DefaultResources: {mode}")
+            raise ValueError(f"Unexpected mode for DefaultResources: {mode}") from None

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

903-903: Fix typo in docstring.

There's a spelling error in the documentation.

-        """Parse a series of CLI-style string resource assigments.
+        """Parse a series of CLI-style string resource assignments.

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 903-903:
assigments ==> assignments

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

src/snakemake/resources.py (1)

599-600: 🛠️ Refactor suggestion

Improve type checking logic for resource constraint

The current implementation has a potential logical issue in the type comparison. It's comparing types with direct equality which may not work correctly for inheritance hierarchies.

    self_val = self.value
-    if not isinstance(self_val, type(other_val)):
+    if not isinstance(self_val, type(other_val)) or not isinstance(other_val, type(self_val)):
        raise ResourceConstraintError(self_val, other_val)

src/snakemake/workflow.py (3)

1755-1756: More efficient resource copying.

Using the copy() method instead of deepcopy() is more efficient and appropriate here as Resources objects don't contain nested mutable objects that would require deep copying.

---

1761-1774: Improved error handling for thread resources.

The try-except block with specific ResourceValidationError handling provides better error reporting. However, the raised exception could be improved.

Apply the following change to improve the exception chain:

        try:
            if overwrite_threads is not None:
                rule.resources["_cores"] = overwrite_threads
            elif ruleinfo.threads is not None:
                rule.resources["_cores"] = ruleinfo.threads
            else:
                rule.resources["_cores"] = 1
        except ResourceValidationError:
-            raise RuleException(
+            raise RuleException(
                "Threads value has to be an integer, float, or a callable.",
                rule=rule,
-            )
+            ) from None

This makes it clear that you're explicitly replacing the original exception with a more user-friendly one.

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

1779-1788: Enhanced resource validation with Resources.from_mapping.

The use of Resources.from_mapping provides structured validation, but the exception handling should be improved.

Apply the following change to improve the exception chain:

            try:
                resources = Resources.from_mapping(resources)
            except KeyError as err:
                raise RuleException(
                    "Resources values have to be integers, strings, or callables "
                    f"(functions): type({err.args[0]} : {err.args[1]}) == "
                    f"{type(err.args[1])}",
                    rule=rule,
-                )
+                ) from err

This preserves the original exception in the chain, making debugging easier.

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

src/snakemake/cli.py (1)

211-229: Consider raising from the original exception for clarity.

When wrapping an exception at line 227, you can preserve the original traceback by adding from e. For example:

-                raise ValueError(f"{errmsg} Cause: {e}")
+                raise ValueError(f"{errmsg} Cause: {e}") from e

This helps distinguish errors in exception handling from the underlying exception.

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

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

(B904)

tests/tests.py (1)

1097-1097: Use a clearer variable name and avoid not l == "\n".

Reading lines with:

lines = [line for line in f if line != "\n"]

would improve readability and satisfy the lint suggestion (!= instead of not ... ==), while also avoiding the single-letter variable name l.

🧰 Tools

🪛 Ruff (0.8.2)

1097-1097: Ambiguous variable name: l

(E741)

---

1097-1097: Use l != "\n" instead of not l == "\n"

Replace with != operator

(SIM201)

src/snakemake/resources.py (7)

4-4: Remove unused imports

There are several imports that are declared but never used in the code:

• dataclasses.dataclass (line 4)
• snakemake.common.get_function_params (line 30)
• snakemake.common.overwrite_function_params (line 34)

-from dataclasses import dataclass
 
 from humanfriendly import InvalidTimespan, InvalidSize, parse_size, parse_timespan

 from snakemake.common import (
-    get_function_params,
     get_input_function_aux_params,
     mb_to_mib,
     mib_to_mb,
-    overwrite_function_params,
 )

Also applies to: 30-30, 34-34

🧰 Tools

🪛 Ruff (0.8.2)

4-4: dataclasses.dataclass imported but unused

Remove unused import: dataclasses.dataclass

(F401)

---

220-220: Use raise ... from err for proper exception chaining

When raising an exception inside an except block, it's a best practice to use raise ... from err to maintain the exception chain for better debugging.

            try:
                layers = cls._get_layers(
                    int_resources, list(sorted_constraints.values()), sortby
                )
            except ResourceError as err:
-                raise ResourceInsufficiencyError(additive_resources, err.args[0])
+                raise ResourceInsufficiencyError(additive_resources, err.args[0]) from err

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

479-479: Fix spelling errors in docstrings

There are several spelling errors in the docstrings:

    """Standardized representation of a resource name and value.

    In addition to ``int`` and ``str``, the standard resource types, ``Resource`` can
    store ``None``, signifying an unset resource, and a callable, signifying a resource
-    to be calculated in the context of a rule. Via the ``.value`` property, ``Resource``
-    ensures a resource has been computed before its value is retreived.
+    to be calculated in the context of a rule. Via the ``.value`` property, ``Resource``
+    ensures a resource has been computed before its value is retrieved.

    The class additionally handles input validation and standardization.
    ``float``s get rounded into ``int``. If the resource name corresponds to a size
    resource ("disk" and "mem") or a timespan resource ("runtime"), strings values
-    will be intepreted as a human readable resource and converted to an integer of
+    will be interpreted as a human readable resource and converted to an integer of
    appropriate denomination (Mb for size, hr for timespan).

    """Evaluate the resource with given args.

    This method ensures the returned resource is correctly returned in a
    new ``Resource`` class, validated, and standardized. Resources should thus
-    only be evaulated via this method.
+    only be evaluated via this method.
    """

    """Create a new evaluable resource based on a python expression.

    Threads can optionally be included in the expression environment

    Arguments
    =========
    name:
        name of the resource
    value:
-        python expression to be evaulated
+        python expression to be evaluated
    with_threads_arg: boolean
        If True, include ``threads`` as an argument in the returned function
    """

-    """Parse a series of CLI-style string resource assigments.
+    """Parse a series of CLI-style string resource assignments.

    Arguments
    =========
    exprs: list of str
        Each expr is in the form RESOURCE=VALUE, where RESOURCE is a valid python
        identifier, and VALUE is a number, a quoted string, or a valid python
        expression.

Also applies to: 484-484, 572-572, 690-690, 911-911

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 479-479:
retreived ==> retrieved

---

420-422: Add more context to resource error message

The error message when a resource constraint is exceeded could be more informative to help users understand what went wrong.

                error_text = [
                    f"\t{res}: {amount}/{constraint}"
                    for res, amount, constraint in too_high
                ]
-                raise ResourceError("\n".join(error_text))
+                raise ResourceError(f"Resource requirement exceeds constraint:\n" + "\n".join(error_text))

---

537-541: Simplify condition with ternary operator

This can be simplified using a ternary operator for better readability.

    def __repr__(self):
-        if self.is_evaluable():
-            value = "function(...)"
-        else:
-            value = self._value
+        value = "function(...)" if self.is_evaluable() else self._value
        return f'Resource("{self.name}", {value})'

🧰 Tools

🪛 Ruff (0.8.2)

537-540: Use ternary operator value = "function(...)" if self.is_evaluable() else self._value instead of if-else-block

Replace if-else-block with value = "function(...)" if self.is_evaluable() else self._value

(SIM108)

---

758-769: Use raise ... from e for proper exception chaining

When raising a new exception inside an except block, using raise ... from e maintains the exception chain for better debugging.

            raise WorkflowError(
                "Failed to evaluate resources value "
                f"'{val}'.\n"
                "    String arguments may need additional "
                "quoting. E.g.: --default-resources "
                "\"tmpdir='/home/user/tmp'\" or "
                "--set-resources \"somerule:someresource='--nice=100'\". "
                "This also holds for setting resources inside of a profile, where "
                "you might have to enclose them in single and double quotes, "
                "i.e. someresource: \"'--nice=100'\".",
-                e,
+                e,
+            ) from e

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

---

725-770: Ensure consistent error handling for resource evaluation

The cli_evaluator method catches exceptions during resource evaluation but uses different approaches for different error types. It would be better to have a consistent approach to exception handling.

        try:
            value = eval(
                val,
                {
                    "input": input,
                    "attempt": attempt,
                    "system_tmpdir": tempfile.gettempdir(),
                    **({"threads": threads} if threads is not None else {}),
                },
            )
        # Triggers for string arguments like n1-standard-4
        except NameError:
            return val
        except Exception as e:
            try:
                # check if resource is parsable as human friendly (given the correct
                # name and formatted value). If it is, we return the parsed value to
                # save a step later.
                if name in HumanFriendlyResources:
                    return Resource._parse_human_friendly(name, val)
-            except (InvalidSize, InvalidTimespan):
+            except (InvalidSize, InvalidTimespan) as parse_err:
                pass
            if is_file_not_found_error(e, input):
                return TBDString()

            raise WorkflowError(
                "Failed to evaluate resources value "
                f"'{val}'.\n"
                "    String arguments may need additional "
                "quoting. E.g.: --default-resources "
                "\"tmpdir='/home/user/tmp'\" or "
                "--set-resources \"somerule:someresource='--nice=100'\". "
                "This also holds for setting resources inside of a profile, where "
                "you might have to enclose them in single and double quotes, "
                "i.e. someresource: \"'--nice=100'\".",
                e,
-            )
+            ) from e

🧰 Tools

🪛 Ruff (0.8.2)

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

(B904)

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

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

106-112: ⚠️ Potential issue

Consider preserving the exception chain for better debugging.

While the change to NestedCoroutineError is correct, consider using raise ... from e to preserve the exception chain as suggested by static analysis:

-        raise NestedCoroutineError(
+        raise NestedCoroutineError(
             "Error running coroutine in event loop. Snakemake currently does not "
             "support being executed from an already running event loop. "
             "If you run Snakemake e.g. from a Jupyter notebook, make sure to spawn a "
             "separate process for Snakemake.",
-            e,
-        )
+        ) from e

🧰 Tools

🪛 Ruff (0.11.9)

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

(B904)

src/snakemake/workflow.py (2)

783-795: 🛠️ Refactor suggestion

Fix exception chaining for better error diagnostics.

The exception handling pattern is correct, but should use proper exception chaining for better debugging.

Apply this diff to improve exception chaining:

-            except ResourceValidationError:
+            except ResourceValidationError as e:
                 raise RuleException(
                     "Threads value has to be an integer, float, or a callable.",
                     rule=rule,
-                )
+                ) from e

---

797-810: 🛠️ Refactor suggestion

Fix exception chaining and approve the resource mapping approach.

The use of Resources.from_mapping() is the correct approach for converting rule resources. However, the exception handling needs proper chaining.

Apply this diff to improve exception chaining:

                 except KeyError as err:
                     raise RuleException(
                         "Resources values have to be integers, strings, or callables "
                         f"(functions): type({err.args[0]} : {err.args[1]}) == "
                         f"{type(err.args[1])}",
                         rule=rule,
-                    )
+                    ) from err

tests/tests.py (1)

2516-2516: ⚠️ Potential issue

Update to use the new Resources.parse pattern for consistency.

This line still uses the old DefaultResources class instead of the new Resources.parse pattern used throughout the rest of the file.

Apply this diff to maintain consistency with the resource system refactoring:

-        default_resources=DefaultResources(["mem_mb=120"]),
+        default_resources=Resources.parse(
+            ["mem_mb=120"], defaults="full", allow_expressions=True
+        ),

🧰 Tools

🪛 Ruff (0.11.9)

2516-2516: Undefined name DefaultResources

(F821)

src/snakemake/rules.py (1)

1089-1089: Fix spelling error.

"inital" should be "initial" in the comment.

-                # dag construction routine are run as independent asynchronous loops.
+                # dag construction routine are run as independent asynchronous loops.

Wait, let me check the actual line again. The error is "inital" -> "initial":

-                # If input.size_mb is run in an input method, the loops will be nested
+                # If input.size_mb is run in an input method, the loops will be nested

Actually, looking at line 1089-1091:

-                # Need to catch this because both input.size_mb and the inital
+                # Need to catch this because both input.size_mb and the initial

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 1089-1089:
inital ==> initial

src/snakemake/cli.py (1)

226-236: Good enhancement but improve exception handling.

The wrapper function support is well-implemented and maintains backward compatibility. However, consider improving exception chaining as suggested by static analysis.

Apply this diff to improve exception handling:

             if wrapper is None:
-                raise ValueError(errmsg)
+                raise ValueError(errmsg) from None

             if wrapper is not None:
                 try:
                     assignments[key] = wrapper(value)
                 except Exception as e:
-                    raise ValueError(f"{errmsg} Cause: {e}")
+                    raise ValueError(f"{errmsg} Cause: {e}") from e

🧰 Tools

🪛 Ruff (0.11.9)

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

(B904)

---

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

(B904)

src/snakemake/jobs.py (1)

37-37: LGTM on import updates, but clean up unused import.

The import changes align well with the resource management refactoring, introducing the new ResourceList, GroupResources, Resources, and ResourceInsufficiencyError classes.

However, there's an unused import that should be removed:

-from snakemake.io import (
-    _IOFile,
-    IOFile,
-    ResourceList,
-    is_callable,
-    Wildcards,
-    is_flagged,
-    get_flag_value,
-    wait_for_files,
-)
+from snakemake.io import (
+    _IOFile,
+    IOFile,
+    ResourceList,
+    Wildcards,
+    is_flagged,
+    get_flag_value,
+    wait_for_files,
+)

Also applies to: 45-45, 51-51

tests/tests.py (2)

1146-1146: Fix code quality issues: ambiguous variable name and comparison style.

Two improvements needed:

1. The variable name l is ambiguous and can be confused with 1 or I
2. Use != instead of not == for better readability

-lines = [l for l in f.readlines() if not l == "\n"]
+lines = [line for line in f.readlines() if line != "\n"]

🧰 Tools

🪛 Ruff (0.11.9)

1146-1146: Ambiguous variable name: l

(E741)

---

1146-1146: Use l != "\n" instead of not l == "\n"

Replace with != operator

(SIM201)

---

1146-1147: Improve variable naming and comparison style.

The static analysis tools correctly identified some style issues:

1. The variable name l is ambiguous and could be confused with 1 or I
2. The comparison not l == "\n" is less clear than using !=

Apply this diff to improve readability:

-        lines = [l for l in f.readlines() if not l == "\n"]
+        lines = [line for line in f.readlines() if line != "\n"]

🧰 Tools

🪛 Ruff (0.11.9)

1146-1146: Ambiguous variable name: l

(E741)

---

1146-1146: Use l != "\n" instead of not l == "\n"

Replace with != operator

(SIM201)

src/snakemake/resources.py (3)

472-494: Fix spelling errors in docstring.

Multiple spelling errors need correction in the class docstring.

    """Standardized representation of a resource name and value.

    In addition to ``int`` and ``str``, the standard resource types, ``Resource`` can
    store ``None``, signifying an unset resource, and a callable, signifying a resource
-    to be calculated in the context of a rule. Via the ``.value`` property, ``Resource``
-    ensures a resource has been computed before its value is retreived.
+    to be calculated in the context of a rule. Via the ``.value`` property, ``Resource``
+    ensures a resource has been computed before its value is retrieved.

    The class additionally handles input validation and standardization.
    ``float``s get rounded into ``int``. If the resource name corresponds to a size
    resource ("disk" and "mem") or a timespan resource ("runtime"), strings values
-    will be intepreted as a human readable resource and converted to an integer of
+    will be interpreted as a human readable resource and converted to an integer of
    appropriate denomination (Mb for size, hr for timespan).

    Arguments
    =========
    name: str
        Name of the resource (note that "disk", "mem", and "runtime" has special
        behaviour, as described above).
    value: int, str, Callable, None
        The concrete or unevaluated value
    """

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 478-478:
retreived ==> retrieved

---

[failure] 483-483:
intepreted ==> interpreted

🪛 GitHub Actions: Codespell

[error] 478-478: codespell: 'retreived' is a misspelling of 'retrieved'

---

681-698: Fix spelling error in docstring.

    @classmethod
    def from_cli_expression(
        cls, name: str, value: str, *, with_threads_arg: bool = True
    ):
        """Create a new evaluable resource based on a python expression.

        Threads can optionally be included in the expression environment

        Arguments
        =========
        name:
            name of the resource
        value:
-            python expression to be evaulated
+            python expression to be evaluated
        with_threads_arg: boolean
            If True, include ``threads`` as an argument in the returned function
        """

🧰 Tools

🪛 GitHub Check: Check for spelling errors

[failure] 694-694:
evaulated ==> evaluated

---

1-1166: Outstanding refactoring of the resource management system!

This comprehensive rewrite significantly improves Snakemake's resource handling:

• Type safety: Strong typing throughout with proper validation
• Human-friendly: Support for parsing "60 Gb", "2 hours" etc.
• Unit conversion: Seamless handling of MB/MiB conversions
• Error handling: Clear, specific exceptions with helpful messages
• Clean architecture: Well-separated concerns between Resource, Resources, and ResourceScopes

The implementation successfully addresses all the bugs mentioned in the PR objectives.

Would you like me to help with any documentation updates or additional test cases for edge cases?

🧰 Tools

🪛 Ruff (0.11.9)

4-4: dataclasses.dataclass imported but unused

Remove unused import: dataclasses.dataclass

(F401)

---

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

(B904)

---

536-539: Use ternary operator value = "function(...)" if self.is_evaluable() else self._value instead of if-else-block

Replace if-else-block with value = "function(...)" if self.is_evaluable() else self._value

(SIM108)

---

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

(B904)

---

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

(B904)

---

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

(B904)

🪛 GitHub Check: Check for spelling errors

[failure] 478-478:
retreived ==> retrieved

---

[failure] 483-483:
intepreted ==> interpreted

---

[failure] 694-694:
evaulated ==> evaluated

---

[failure] 918-918:
assigments ==> assignments

🪛 GitHub Actions: Codespell

[error] 478-478: codespell: 'retreived' is a misspelling of 'retrieved'

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