v0.10.1 (#69)

* Close #62

* Update CHANGELOG

* Update pre-commit config in project template

* Close #68

* Update CHANGELOG

* Close #64

* Close #65

CHANGELOG.md

@@ -8,11 +8,20 @@ and this project adheres to [Semantic Versioning][].
 [keep a changelog]: https://keepachangelog.com/en/1.0.0/
 [semantic versioning]: https://semver.org/spec/v2.0.0.html
 
-## [Unreleased]
+## v0.10.1
+
+### Fixes
+
+-   Take comments into account when linting for `DSO001` ([#69](https://github.com/Boehringer-Ingelheim/dso/pull/69))
+-   Make it possible to override watermarks/disclaimers with a simple `null` ([#69](https://github.com/Boehringer-Ingelheim/dso/pull/69)).
+-   Compile *all* configs on `dso repro`, not just the ones relvant to the specified stage. This is required because we don't
+    know which other stages dvc might compile ([#69](https://github.com/Boehringer-Ingelheim/dso/pull/69)).
+-   Make `get-config` compatible with dvc matrix stages ([#69](https://github.com/Boehringer-Ingelheim/dso/pull/69)).
 
 ### Template updates
 
--   Do not ignore gitignore files in output/report directories of template ([#63](https://github.com/Boehringer-Ingelheim/dso/pull/63))
+-   Do not ignore the `.gitignore` files in output/report directories of template ([#63](https://github.com/Boehringer-Ingelheim/dso/pull/63))
+-   Update `.pre-commit-config.yaml` for pre-commit 4.x ([#63](https://github.com/Boehringer-Ingelheim/dso/pull/63))
 
 ## v0.10.0
 

src/dso/get_config.py

@@ -99,6 +99,7 @@ def get_config(stage: str, *, all: bool = False, skip_compile: bool = False) ->
 
         # We want to include parameters mentioned in either `params`, `deps`, `outs`.
         # The parameters in `deps`/`outs` are encapsulated in `${ <param> }`
+        is_matrix_stage = "matrix" in dvc_stage_config
         keep_params = set(dvc_stage_config.get("params", []))
         dvc_param_pat = re.compile(r"\$\{\s*(.*?)\s*\}")
         for dep in dvc_stage_config.get("deps", []):
@@ -112,6 +113,9 @@ def get_config(stage: str, *, all: bool = False, skip_compile: bool = False) ->
             f"Only including the following parameters which are listed in `dvc.yaml`: [green]{', '.join(keep_params)}"
         )
 
+        if is_matrix_stage:
+            keep_params = {p for p in keep_params if not (p.startswith("item.") or p == "item")}
+
         return _filter_nested_dict(config, keep_params)
 
 

src/dso/lint.py

@@ -87,6 +87,15 @@ def check(cls, file):
         # .parent to remove the dvc.yaml filename
         stage_path_expected = str(stage_path_expected.parent.relative_to(root_path))
         content = file.read_text()
+
+        # remove comments
+        # TODO there are still edge cases, e.g. a `#` within a string doesn't initiate a comment
+        # However this is hard/impossible (?) to solve with a regular expression alone, we'd need a proper
+        # R parse to address all edge cases. See also https://github.com/Boehringer-Ingelheim/dso/issues/66
+        pattern_is_comment = re.compile(r"#.*$")
+        content = "\n".join([re.sub(pattern_is_comment, "", line) for line in content.split("\n")])
+
+        # detect pattern
         pattern = r"[\s\S]*?(dso::)?read_params\s*\(([\s\S]*?)(\s*,.*)?\)"
         res = re.findall(pattern, content, flags=re.MULTILINE)
         if len(res) == 0:

src/dso/pandocfilter.py

@@ -75,7 +75,7 @@ def _sanitize_watermark_config(config):
 def action(elem, doc):
     """Panflutes action"""
     watermark_config = _sanitize_watermark_config(doc.get_metadata("watermark"))
-    if watermark_config is not None:
+    if watermark_config:  # could be "" or None, both which evaluate to False
         if "text" not in watermark_config:
             log.error("Need to specify at least `watermark.text`")
             sys.exit(1)

src/dso/repro.py

@@ -6,7 +6,7 @@
 import rich_click as click
 
 from dso._logging import log
-from dso._util import check_ask_pre_commit
+from dso._util import check_ask_pre_commit, get_project_root
 from dso.compile_config import compile_all_configs
 
 
@@ -18,7 +18,7 @@
 def cli(args):
     """Wrapper around dvc repro, compiling configuration before running."""
     check_ask_pre_commit(Path.cwd())
-    compile_all_configs([Path.cwd()])
+    compile_all_configs([get_project_root(Path.cwd())])
     os.environ["DSO_SKIP_COMPILE"] = "1"
     cmd = ["dvc", "repro", *args]
     log.info(f"Running `{' '.join(cmd)}`")

src/dso/templates/init/default/.pre-commit-config.yaml

@@ -43,12 +43,12 @@ repos:
       - id: dvc-pre-commit
         language_version: python3
         stages:
-          - commit
+          - pre-commit
       - id: dvc-pre-push
         additional_dependencies: ["s3"]
         language_version: python3
         stages:
-          - push
+          - pre-push
         verbose: true
       - id: dvc-post-checkout
         always_run: true
@@ -62,9 +62,9 @@ repos:
         name: Run dso compile-config
         entry: dso compile-config
         language: system
-        stages: [commit]
+        stages: [pre-commit]
       - id: lint
         name: Run dso lint
         entry: dso lint --skip-compile
         language: system
-        stages: [commit]
+        stages: [pre-commit]

tests/test_get_config.py

@@ -142,6 +142,44 @@ def test_get_config_order(dso_project):
     assert list(config["B1"]) == ["C", "A", "D", "B", "Z", "X"]
 
 
+def test_get_config_matrix(dso_project):
+    """Test that get-config is compatible with dvc's matrix feature"""
+    chdir(dso_project)
+    stage = dso_project / "mystage"
+    stage.mkdir()
+    (stage / "params.in.yaml").touch()
+
+    (dso_project / "params.in.yaml").write_text(
+        dedent(
+            """\
+            matrix_param: ['p1', 'p2']
+            A: "aaa"
+            B: "bbb"
+            """
+        )
+    )
+
+    (stage / "dvc.yaml").write_text(
+        dedent(
+            """\
+            stages:
+               mystage01:
+                 matrix:
+                    mp: ${ matrix_param }
+                 params:
+                   - A
+                   - item.mp
+                 outs:
+                   - output/${ item.mp }
+                 cmd: "echo Hello World!"
+            """
+        )
+    )
+
+    config = get_config("mystage")
+    assert list(config) == ["A"]
+
+
 def test_get_config_path_relative_to_root_dir(quarto_stage):
     chdir(quarto_stage)
     config1 = get_config("quarto_stage")

tests/test_lint.py

@@ -116,6 +116,21 @@ class MockQuartoRule(QuartoRule):
             """,
             LintError,
         ),
+        (
+            """\
+            params = read_params("quarto_stage")
+            # params = read_params("quarto_stage")
+            """,
+            None,
+        ),
+        # TODO no good way to cover that with regex alone, see https://github.com/Boehringer-Ingelheim/dso/issues/66
+        # (
+        #     """\
+        #     params = read_params("quarto_stage")
+        #     print(" foo # no comment"); params = read_params("quarto_stage")
+        #     """,
+        #     LintError,
+        # ),
         (
             """\
             params = read_params("wrong_path")

tests/test_pandocfilter.py

@@ -1,7 +1,10 @@
+from os import chdir
 from shutil import copyfile
 from textwrap import dedent
 
-from dso.exec import _render_quarto
+from click.testing import CliRunner
+
+from dso.exec import _render_quarto, cli
 from tests.conftest import TESTDATA
 
 
@@ -63,3 +66,42 @@ def test_pandocfilter(quarto_stage):
     assert "Disclaimer" in out_html
     assert "This is a disclaimer" in out_html
     assert "callout-important" in out_html
+
+
+def test_override_config(quarto_stage):
+    """Test that it's possible to remove a watermark/disclaimer by overriding the config with null"""
+    # I didn't find a straightforward way of testing programmatically that there's really no watermark.
+    # this test still guarantees that it doesn't fail with an error when overring the watermark config (which it did previously)
+    (quarto_stage / ".." / "params.in.yaml").write_text(
+        dedent(
+            """\
+            dso:
+              quarto:
+                watermark:
+                  text: test
+                disclaimer:
+                  title: test disclaimer
+                  text: lorem ipsum
+            """
+        )
+    )
+    (quarto_stage / "params.in.yaml").write_text(
+        dedent(
+            """\
+            dso:
+              quarto:
+                watermark: null
+                disclaimer: null
+            """
+        )
+    )
+
+    runner = CliRunner()
+    chdir(quarto_stage)
+    stage_path = "."
+
+    result = runner.invoke(cli, ["quarto", stage_path])
+    assert result.exit_code == 0
+
+    out_html = (quarto_stage / "report" / "quarto_stage.html").read_text()
+    assert "test disclaimer" not in out_html