diff --git a/dandi/bids_validator_deno/_validator.py b/dandi/bids_validator_deno/_validator.py index d97f953db..7e28bb7cc 100644 --- a/dandi/bids_validator_deno/_validator.py +++ b/dandi/bids_validator_deno/_validator.py @@ -12,8 +12,10 @@ from packaging.version import parse as parse_ver_str from pydantic import DirectoryPath, validate_call +from dandi.consts import dandiset_metadata_file from dandi.utils import find_parent_directory_containing from dandi.validate_types import ( + ORIGIN_VALIDATION_DANDI_LAYOUT, Origin, OriginType, Scope, @@ -361,20 +363,45 @@ def _harmonize( # The absolute path to the file or directory that the issue is related to issue_path = _get_path(issue, ds_path) - results.append( - ValidationResult( - id=f"BIDS.{issue.code}", - origin=origin, - scope=_get_scope(issue_path), - # Store only the issue, not entire bv_result with more context - origin_result=issue, - severity=_SEVERITY_MAP[severity] if severity else None, + validation_result = ValidationResult( + id=f"BIDS.{issue.code}", + origin=origin, + scope=_get_scope(issue_path), + # Store only the issue, not entire bv_result with more context + origin_result=issue, + severity=_SEVERITY_MAP[severity] if severity else None, + dandiset_path=dandiset_path, + dataset_path=ds_path, + message=_get_msg(issue, code_messages), + path=issue_path, + ) + results.append(validation_result) + + # If the error is about the dandiset metadata file, add a HINT + # suggesting to add it to .bidsignore + is_dandiset_yaml_error = ( + issue_path is not None + and issue_path.relative_to(ds_path).as_posix() == dandiset_metadata_file + ) + if is_dandiset_yaml_error: + hint = ValidationResult( + id="DANDI.BIDSIGNORE_DANDISET_YAML", + origin=ORIGIN_VALIDATION_DANDI_LAYOUT, + scope=Scope.DATASET, + origin_result=validation_result, + severity=Severity.HINT, dandiset_path=dandiset_path, dataset_path=ds_path, - message=_get_msg(issue, code_messages), path=issue_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}" + ), ) - ) + results.append(hint) return results diff --git a/dandi/tests/test_validate.py b/dandi/tests/test_validate.py index 5b240eff8..703de8730 100644 --- a/dandi/tests/test_validate.py +++ b/dandi/tests/test_validate.py @@ -114,11 +114,30 @@ 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 + # The error message should be the original BIDS error, not modified 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." + # We just check that it's about dandiset.yaml, not checking exact message + # since it comes from BIDS validator + + # Check that there is also a HINT about .bidsignore + validation_hints = [ + r + for r in validation_results + if r.severity is not None and r.severity == Severity.HINT + ] + + # Assert that there is at least one hint + assert len(validation_hints) >= 1 + + # Find the hint about .bidsignore for dandiset.yaml + bidsignore_hint = next( + (h for h in validation_hints if h.id == "DANDI.BIDSIGNORE_DANDISET_YAML"), + None, ) + assert bidsignore_hint is not None + assert bidsignore_hint.message is not None + assert ".bidsignore" in bidsignore_hint.message + assert dandiset_metadata_file in bidsignore_hint.message def test_validate_bids_onefile(bids_error_examples: Path, tmp_path: Path) -> None: diff --git a/dandi/validate.py b/dandi/validate.py index e67dfb383..3b4dae26f 100644 --- a/dandi/validate.py +++ b/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