fix: Restore python rules changes triggering reruns. (GH: #3213) (#3485)

royjacobson · web-flow · commit 2f663be3f272 · 2025-09-04T21:08:37.000+02:00
Trying to fix #3213 by caching the source code of python rules (using `dis` + `linecache` magic). I would appreciate any help in how to approach writing a test for this. cc @corneliusroemer ### QC  * [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).  ## Summary by CodeRabbit - **New Features** - Enhanced rule processing to capture both shell command and Python function definitions when available. - Expanded the internal rule configuration to include additional source code information, supporting improved debugging and documentation. - Introduced a method to retrieve and store source details for rules automatically. - New workflow defined with rules for generating files based on time-stamped content. - **Bug Fixes** - Added tests to validate the behavior of the new rule processing and ensure expected outputs.
diff --git a/src/snakemake/persistence.py b/src/snakemake/persistence.py
@@ -564,11 +564,13 @@ def _b64id(self, s):
 
     @lru_cache()
     def _code(self, rule):
-        # We only consider shell commands for now.
-        # Plain python code rules are hard to capture because the pickling of the code
-        # can change with different python versions.
         # Scripts and notebooks are triggered by changes in the script mtime.
-        return rule.shellcmd if rule.shellcmd is not None else None
+        # Changes to python and shell rules are triggered by changes in the plain text.
+        if rule.shellcmd is not None:
+            return rule.shellcmd
+        if rule.run_func_src is not None:
+            return rule.run_func_src
+        return None
 
     @lru_cache()
     def _conda_env(self, job):
diff --git a/src/snakemake/rules.py b/src/snakemake/rules.py
@@ -108,6 +108,7 @@ def __init__(self, name, workflow, lineno=None, snakefile=None):
         self._lineno = lineno
         self._snakefile = snakefile
         self.run_func = None
+        self.run_func_src = None
         self.shellcmd = None
         self.script = None
         self.notebook = None
diff --git a/src/snakemake/workflow.py b/src/snakemake/workflow.py
@@ -10,6 +10,8 @@
 import subprocess
 import sys
 import platform
+import dis
+import linecache
 from collections import OrderedDict, namedtuple
 from collections.abc import Iterator, Mapping
 from itertools import filterfalse, chain
@@ -1762,6 +1764,26 @@ def ruleorder(self, *rulenames):
     def localrules(self, *rulenames):
         self._localrules.update(rulenames)
 
+    def get_rule_source(self, func):
+        # This can't use `inspect` because the functions are compiled into intermediate python code
+        # in parser.py and that intermediate source is not available anymore (or desirable).
+        # Instead, we're using dis to retrieve the line numbers of the function in the intermediate
+        # code and then map it to the original file using `self.linemaps`.
+        sourcefile = func.__code__.co_filename
+        line_numbers = []
+        linemap = self.linemaps[sourcefile]
+        for func_offset, line in dis.findlinestarts(func.__code__):
+            # The first instruction in the compiled function is RESUME, which
+            # with snakemake is mapped to the 'rule: ' line and is not considered
+            # part of the rule source.
+            if func_offset == 0:
+                continue
+            if line in linemap:
+                line_numbers.append(linemap[line])
+        return "".join(
+            [linecache.getline(sourcefile, lineno) for lineno in sorted(line_numbers)]
+        )
+
     def rule(self, name=None, lineno=None, snakefile=None, checkpoint=False):
         # choose a name for an unnamed rule
         if name is None:
@@ -1980,6 +2002,8 @@ def check_may_use_software_deployment(method):
                 rule.name = ruleinfo.name
             rule.docstring = ruleinfo.docstring
             rule.run_func = ruleinfo.func
+            if rule.run_func is not None and not rule.norun:
+                rule.run_func_src = self.get_rule_source(rule.run_func)
             rule.shellcmd = ruleinfo.shellcmd
             rule.script = ruleinfo.script
             rule.notebook = ruleinfo.notebook
diff --git a/tests/test_github_issue3213/Snakefile b/tests/test_github_issue3213/Snakefile
@@ -0,0 +1,23 @@
+rule all:
+    input:
+        "file_b.txt"
+
+rule file_a:
+    output:
+        "file_a.txt"
+    run:
+        import time
+
+        with open(output[0], "w") as f:
+            f.write("A" + str(time.time_ns()))
+
+rule file_b:
+    input:
+        "file_a.txt"
+    output:
+        "file_b.txt"
+    run:
+        import time
+
+        with open(output[0], "w") as f:
+            f.write("B" + str(time.time_ns()))
diff --git a/tests/test_github_issue3213/Snakefile.newver b/tests/test_github_issue3213/Snakefile.newver
@@ -0,0 +1,23 @@
+rule all:
+    input:
+        "file_b.txt"
+
+rule file_a:
+    output:
+        "file_a.txt"
+    run:
+        import time
+
+        with open(output[0], "w") as f:
+            f.write("A" + str(time.time_ns()))
+
+rule file_b:
+    input:
+        "file_a.txt"
+    output:
+        "file_b.txt"
+    run:
+        import time
+
+        with open(output[0], "w") as f:
+            f.write("D" + str(time.time_ns()))
diff --git a/tests/tests.py b/tests/tests.py
@@ -1977,6 +1977,52 @@ def test_github_issue1882():
         shutil.rmtree(tmpdir)
 
 
+def test_github_issue3213():
+    try:
+        import linecache
+
+        output_files = ["file_a.txt", "file_b.txt"]
+
+        tmpdir = run(dpath("test_github_issue3213"), cleanup=False, check_results=False)
+        ts_a1, ts_b1 = [open(os.path.join(tmpdir, f)).read() for f in output_files]
+
+        # Need to clear the cache so that we notice the Snakefile file has changed.
+        # In practice it's not a problem.
+        linecache.clearcache()
+
+        shutil.copy(
+            os.path.join(dpath("test_github_issue3213"), "Snakefile.newver"),
+            os.path.join(tmpdir, "Snakefile"),
+        )
+        run(
+            dpath("test_github_issue3213"),
+            tmpdir=tmpdir,
+            cleanup=False,
+            check_results=False,
+        )
+        ts_a2, ts_b2 = [open(os.path.join(tmpdir, f)).read() for f in output_files]
+
+        run(
+            dpath("test_github_issue3213"),
+            tmpdir=tmpdir,
+            cleanup=False,
+            check_results=False,
+            forceall=True,
+        )
+        ts_a3, ts_b3 = [open(os.path.join(tmpdir, f)).read() for f in output_files]
+
+        assert ts_a1[0] == ts_a2[0] == ts_a3[0] == "A"
+        assert ts_b1[0] == "B"
+        assert ts_b2[0] == ts_b3[0] == "D"
+
+        assert ts_a1 == ts_a2
+        assert ts_b1 != ts_b2
+        assert ts_a2 != ts_a3
+        assert ts_b2 != ts_b3
+    finally:
+        shutil.rmtree(tmpdir)
+
+
 @skip_on_windows  # not platform dependent
 def test_inferred_resources():
     run(dpath("test_inferred_resources"))
