From 957da049abd6589ce7810afc5ac9f5da2c247880 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 12 Nov 2025 21:02:22 +0000 Subject: [PATCH 1/3] Add HINT validation result for dandiset.yaml in BIDS datasets When BIDS validator encounters dandiset.yaml (not part of BIDS spec), now: - Keeps the existing ERROR about the file not matching BIDS patterns - Adds a new HINT level validation result suggesting to create/update .bidsignore Includes test to verify both ERROR and HINT are generated Co-authored-by: yarikoptic <39889+yarikoptic@users.noreply.github.com> --- dandi/tests/test_validate.py | 20 ++++++++++++++++++++ dandi/validate.py | 26 ++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/dandi/tests/test_validate.py b/dandi/tests/test_validate.py index 5b240eff8..45be0abac 100644 --- a/dandi/tests/test_validate.py +++ b/dandi/tests/test_validate.py @@ -120,6 +120,26 @@ def mock_bids_validate(*args: Any, **kwargs: Any) -> list[ValidationResult]: f"BIDS specification." ) + # 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..7b9e7581b 100644 --- a/dandi/validate.py +++ b/dandi/validate.py @@ -188,12 +188,14 @@ def validate( 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 ( + # and add a HINT to suggest adding to .bidsignore + is_dandiset_yaml_error = ( 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 - ): + ) + if is_dandiset_yaml_error: r.message = ( f"The dandiset metadata file, `{dandiset_metadata_file}`, " f"is not a part of BIDS specification. Please include a " @@ -205,3 +207,23 @@ def validate( df_results.append(r) df_result_ids.add(r_id) yield r + + # After yielding the error for dandiset.yaml, also yield a HINT + if is_dandiset_yaml_error: + hint = ValidationResult( + id="DANDI.BIDSIGNORE_DANDISET_YAML", + origin=ORIGIN_VALIDATION_DANDI_LAYOUT, + severity=Severity.HINT, + scope=Scope.DATASET, + path=r.dataset_path, + dataset_path=r.dataset_path, + dandiset_path=dandiset_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}" + ), + ) + yield hint From 6faecd2ab0592800d811c616a6879d771d29ff0b Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Wed, 7 Jan 2026 12:18:33 -0500 Subject: [PATCH 2/3] RF: do not adjust existing message, add a HINT one and adjust record per Isaac review --- dandi/validate.py | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/dandi/validate.py b/dandi/validate.py index 7b9e7581b..0014f8bda 100644 --- a/dandi/validate.py +++ b/dandi/validate.py @@ -186,8 +186,12 @@ 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 + df_results.append(r) + df_result_ids.add(r_id) + yield r + + # If the error is about the dandiset metadata file, add a HINT + # message in the validation to give the context of DANDI # and add a HINT to suggest adding to .bidsignore is_dandiset_yaml_error = ( r.path is not None @@ -195,29 +199,16 @@ def validate( and r.path.relative_to(r.dataset_path).as_posix() == dandiset_metadata_file ) - if is_dandiset_yaml_error: - 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 - - # After yielding the error for dandiset.yaml, also yield a HINT if is_dandiset_yaml_error: hint = ValidationResult( id="DANDI.BIDSIGNORE_DANDISET_YAML", origin=ORIGIN_VALIDATION_DANDI_LAYOUT, - severity=Severity.HINT, scope=Scope.DATASET, - path=r.dataset_path, + origin_result=r, + severity=Severity.HINT, + dandiset_path=r.dandiset_path, dataset_path=r.dataset_path, - dandiset_path=dandiset_path, + path=r.path, message=( f"Consider creating or updating a `.bidsignore` file " f"in the root of your BIDS dataset to ignore " From bbe7870b536c5bc8da7a94408f52f140e9e0d4cd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 8 Jan 2026 00:42:52 +0000 Subject: [PATCH 3/3] Move HINT generation to BIDS validator harmonization - Moved dandiset.yaml HINT logic from validate() to _harmonize() in bids_validator_deno - This ensures HINT appears for both 'dandi validate' and 'dandi upload' commands - Removed modification of original BIDS error message (preserving original error) - Updated test to not expect modified error message - All 101 validation tests passing Co-authored-by: yarikoptic <39889+yarikoptic@users.noreply.github.com> --- dandi/bids_validator_deno/_validator.py | 47 +++++++++++++++++++------ dandi/tests/test_validate.py | 7 ++-- dandi/validate.py | 29 --------------- 3 files changed, 40 insertions(+), 43 deletions(-) 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 45be0abac..703de8730 100644 --- a/dandi/tests/test_validate.py +++ b/dandi/tests/test_validate.py @@ -114,11 +114,10 @@ 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 = [ diff --git a/dandi/validate.py b/dandi/validate.py index 0014f8bda..3b4dae26f 100644 --- a/dandi/validate.py +++ b/dandi/validate.py @@ -189,32 +189,3 @@ def validate( df_results.append(r) df_result_ids.add(r_id) yield r - - # If the error is about the dandiset metadata file, add a HINT - # message in the validation to give the context of DANDI - # and add a HINT to suggest adding to .bidsignore - is_dandiset_yaml_error = ( - 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 - ) - if is_dandiset_yaml_error: - hint = ValidationResult( - id="DANDI.BIDSIGNORE_DANDISET_YAML", - origin=ORIGIN_VALIDATION_DANDI_LAYOUT, - scope=Scope.DATASET, - origin_result=r, - severity=Severity.HINT, - dandiset_path=r.dandiset_path, - dataset_path=r.dataset_path, - path=r.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}" - ), - ) - yield hint