From 27f3876e6c3b64ec5acd6c993b8d2cdfcd3dfe5e Mon Sep 17 00:00:00 2001
From: bpinsard <basile.pinsard@gmail.com>
Date: Wed, 2 Oct 2024 16:10:14 -0400
Subject: [PATCH] work on the error display + fixes

---
 src/forbids/cli/run.py              | 15 +++---------
 src/forbids/{cli => }/init.py       |  2 +-
 src/forbids/schema.py               |  2 +-
 src/forbids/{cli => }/validation.py | 36 +++++++++++++++++++++++++++--
 4 files changed, 39 insertions(+), 16 deletions(-)
 rename src/forbids/{cli => }/init.py (99%)
 rename src/forbids/{cli => }/validation.py (78%)

diff --git a/src/forbids/cli/run.py b/src/forbids/cli/run.py
index bb5b8f4..9b351b7 100644
--- a/src/forbids/cli/run.py
+++ b/src/forbids/cli/run.py
@@ -6,8 +6,8 @@
 
 import bids
 
-from .init import initialize
-from .validation import validate
+from ..init import initialize
+from ..validation import process_validation
 
 DEBUG = bool(os.environ.get("DEBUG", False))
 if DEBUG:
@@ -68,16 +68,7 @@ def main() -> None:
             version_specific=args.version_specific,
         )
     elif args.command == "validate":
-        no_error = True
-        for error in validate(layout, subject=args.participant_label, session=args.session_label):
-            no_error = False
-            lgr.error(
-                "%s %s : %s found %s",
-                error.__class__.__name__,
-                ".".join([str(e) for e in error.absolute_path]),
-                error.message,
-                error.instance if "required" not in error.message else "",
-            )
+        no_error = process_validation(layout, subject=args.participant_label, session=args.session_label)
         exit(0 if no_error else 1)
 
 
diff --git a/src/forbids/cli/init.py b/src/forbids/init.py
similarity index 99%
rename from src/forbids/cli/init.py
rename to src/forbids/init.py
index b45729a..e90ef3c 100644
--- a/src/forbids/cli/init.py
+++ b/src/forbids/init.py
@@ -13,7 +13,7 @@
 from apischema.json_schema import deserialization_schema
 from jsonschema.exceptions import ValidationError
 
-from .. import schema
+from . import schema
 
 configs = {}
 lgr = logging.getLogger(__name__)
diff --git a/src/forbids/schema.py b/src/forbids/schema.py
index 119f138..0ed0699 100644
--- a/src/forbids/schema.py
+++ b/src/forbids/schema.py
@@ -164,5 +164,5 @@ def prepare_metadata(
     # rename conflictual keywords as the schema was created
     sidecar_data = {k + ("__" if k in keyword.kwlist else ""): v for k, v in sidecar.get_dict().items()}
     # create an aggregate tag of all schema-defined instrument tags
-    sidecar_data["__instrument__"] = "".join([sidecar_data.get(instr_tag, "unknown") for instr_tag in instrument_tags])
+    sidecar_data["__instrument__"] = "-".join([sidecar_data.get(instr_tag, "unknown") for instr_tag in instrument_tags])
     return sidecar_data
diff --git a/src/forbids/cli/validation.py b/src/forbids/validation.py
similarity index 78%
rename from src/forbids/cli/validation.py
rename to src/forbids/validation.py
index c966ecd..a38e177 100644
--- a/src/forbids/cli/validation.py
+++ b/src/forbids/validation.py
@@ -4,13 +4,19 @@
 import os
 
 import bids
+from jsonschema._utils import Unset
 from jsonschema.exceptions import ValidationError
 
-from .. import schema
+from . import schema
 
 lgr = logging.getLogger(__name__)
 
 
+class BIDSJSONError(ValidationError):
+    # class to represent BIDS metadata error
+    pass
+
+
 class BIDSFileError(ValidationError):
     # class to represent error of BIDS file missing or unexpected
     pass
@@ -94,6 +100,32 @@ def validate(bids_layout: bids.BIDSLayout, **entities: dict[str, str | list]):
                         lgr.error("an error occurred")
                     lgr.debug("validating %s", sidecar.relpath)
                     sidecar_data = schema.prepare_metadata(sidecar, bidsfile_constraints["instrument_tags"])
-                    yield from validator.iter_errors(sidecar_data)
+                    yield from add_path_note_to_error(validator, sidecar_data, sidecar.relpath)
     for extra_sidecar in all_sidecars:
         yield BIDSFileError("Unexpected BIDS file %s", extra_sidecar.relpath)
+
+
+def add_path_note_to_error(validator, sidecar_data, filepath):
+    for error in validator.iter_errors(sidecar_data):
+        error.add_note(filepath)
+        yield error
+
+
+def process_validation(layout, subject, session):
+    no_error = True
+    for error in validate(layout, subject=subject, session=session):
+        no_error = False
+
+        formatted_message = error.message
+        if len(error.path) == 0 and not isinstance(error.instance, Unset):
+            formatted_message = f"non-existing schema for instrument {error.instance['__instrument__']}"
+
+        lgr.error(
+            "%s %s %s : %s",
+            error.__class__.__name__,
+            error.__notes__[0] if hasattr(error, "__notes__") else "",
+            ".".join([str(e) for e in error.absolute_path]),
+            formatted_message,
+        )
+        lgr.debug(error)
+    return no_error