Merge pull request #1741 from dandi/copilot/add-hint-for-dandiset-yaml

Add HINT validation result for dandiset.yaml in both validate and upload commands

Author: yarikoptic

dandi/files/bids.py

@@ -13,10 +13,15 @@
 
 from .bases import GenericAsset, LocalFileAsset, NWBAsset
 from .zarr import ZarrAsset
-from ..consts import ZARR_MIME_TYPE
+from ..consts import ZARR_MIME_TYPE, dandiset_metadata_file
 from ..metadata.core import add_common_metadata, prepare_metadata
 from ..misctypes import Digest
-from ..validate_types import ValidationResult
+from ..validate_types import (
+    ORIGIN_VALIDATION_DANDI_LAYOUT,
+    Scope,
+    Severity,
+    ValidationResult,
+)
 
 BIDS_ASSET_ERRORS = ("BIDS.NON_BIDS_PATH_PLACEHOLDER",)
 BIDS_DATASET_ERRORS = ("BIDS.MANDATORY_FILE_MISSING_PLACEHOLDER",)
@@ -112,7 +117,42 @@ def _validate(self) -> None:
 
                 # Obtain BIDS validation results of the entire dataset through the
                 # deno-compiled BIDS validator
-                self._dataset_errors = bids_validate(self.bids_root)
+                v_results = bids_validate(self.bids_root)
+
+                # Validation results from the deno BIDS validator with an additional
+                # hint, represented as a `ValidationResult` object, following
+                # each `dandiset.yaml` error, suggesting to add the `dandiset.yaml` file
+                # to `.bidsignore`.
+                v_results_extended: list[ValidationResult] = []
+
+                for result in v_results:
+                    v_results_extended.append(result)
+                    if (
+                        result.path is not None
+                        and result.dataset_path is not None
+                        and result.path.relative_to(result.dataset_path).as_posix()
+                        == dandiset_metadata_file
+                    ):
+                        hint = ValidationResult(
+                            id="DANDI.BIDSIGNORE_DANDISET_YAML",
+                            origin=ORIGIN_VALIDATION_DANDI_LAYOUT,
+                            scope=Scope.DATASET,
+                            origin_result=result,
+                            severity=Severity.HINT,
+                            dandiset_path=result.dandiset_path,
+                            dataset_path=result.dataset_path,
+                            path=result.path,
+                            message=(
+                                f"Consider creating or updating a `.bidsignore` file "
+                                f"in the root of your BIDS dataset to ignore "
+                                f"`{dandiset_metadata_file}`. "
+                                f"Add the following line to `.bidsignore`:\n"
+                                f"{dandiset_metadata_file}"
+                            ),
+                        )
+                        v_results_extended.append(hint)
+
+                self._dataset_errors = v_results_extended
 
                 # Categorized validation results related to individual assets by the
                 # path of the asset in the BIDS dataset

dandi/tests/test_validate.py

@@ -114,11 +114,18 @@ def mock_bids_validate(*args: Any, **kwargs: Any) -> list[ValidationResult]:
     assert err.dataset_path is not None
     assert err.path.relative_to(err.dataset_path).as_posix() == dandiset_metadata_file
 
-    assert err.message is not None
-    assert err.message.startswith(
-        f"The dandiset metadata file, `{dandiset_metadata_file}`, is not a part of "
-        f"BIDS specification."
-    )
+    # === Assert that there is the dandiset.yaml hint ===
+    i = None
+    for i, r in enumerate(validation_results):
+        if r is err:
+            break
+
+    assert i is not None
+    # There must be at least one more result after the error
+    assert len(validation_results) > i + 1
+    # The next result must be the hint re: dandiset.yaml
+    assert validation_results[i + 1].id == "DANDI.BIDSIGNORE_DANDISET_YAML"
+    assert validation_results[i + 1].severity == Severity.HINT
 
 
 def test_validate_bids_onefile(bids_error_examples: Path, tmp_path: Path) -> None:

dandi/validate.py

@@ -186,22 +186,6 @@ def validate(
             ):
                 r_id = id(r)
                 if r_id not in df_result_ids:
-                    # If the error is about the dandiset metadata file, modify
-                    # the message in the validation to give the context of DANDI
-                    if (
-                        r.path is not None
-                        and r.dataset_path is not None
-                        and r.path.relative_to(r.dataset_path).as_posix()
-                        == dandiset_metadata_file
-                    ):
-                        r.message = (
-                            f"The dandiset metadata file, `{dandiset_metadata_file}`, "
-                            f"is not a part of BIDS specification. Please include a "
-                            f"`.bidsignore` file with specification to ignore the "
-                            f"metadata file in your dataset. For more details, see "
-                            f"https://github.com/bids-standard/bids-specification/"
-                            f"issues/131#issuecomment-461060166."
-                        )
                     df_results.append(r)
                     df_result_ids.add(r_id)
                     yield r