Enhance error notifications

src/nwb2bids/_command_line_interface/_main.py

@@ -1,11 +1,12 @@
+import collections
 import pathlib
 import typing
 
 import rich_click
 
 from .._converters._run_config import RunConfig
 from .._core._convert_nwb_dataset import convert_nwb_dataset
-from .._inspection._inspection_result import Severity
+from .._inspection._inspection_result import InspectionResult, Severity
 from .._tools._pluralize import _pluralize
 from ..testing import generate_ephys_tutorial
 
@@ -112,23 +113,60 @@ def _run_convert_nwb_dataset(
 
     converter = convert_nwb_dataset(nwb_paths=handled_nwb_paths, run_config=run_config)
 
+    if silent:
+        return
+
     notifications = converter.messages
+    notifications_by_severity: dict[Severity, list[InspectionResult]] = collections.defaultdict(list)
+    for notification in notifications:
+        notifications_by_severity[notification.severity].append(notification)
+    errors = notifications_by_severity[Severity.ERROR]
+    criticals = notifications_by_severity[Severity.CRITICAL]
+
     console_notification = ""
-    if notifications:
-        notification_text = (
-            f'\n{(n:=len(notifications))} {_pluralize(n=n, word="suggestion")} for improvement '
-            f'{_pluralize(n=n, word="was", plural="were")} found during conversion.'
-        )
-        console_notification += rich_click.style(text=notification_text, fg="yellow")
+    if errors:
+        number_of_errors = len(errors)
+
+        top_three = errors[:3]
+        number_to_print = len(top_three)
 
-    if console_notification != "" and not silent:
+        text = (
+            "\nBIDS dataset was not successfully created!\n"
+            f'{_pluralize(n=number_to_print, phrase="An error was", plural="Some errors were")} '
+            "encountered during conversion.\n"
+        )
+        if number_to_print > 1 and number_of_errors > 3:
+            error_text = "".join(f"\n\t- {error.reason}" for error in top_three)
+            text += (
+                f"The first {number_to_print} of {number_of_errors} are shown below:\n\n"
+                f"{error_text}\n\n"
+                # TODO: "The full log file can be found at {run_config.log_file_path}\n"
+            )
+        console_notification = rich_click.style(text=text, fg="red")
         rich_click.echo(message=console_notification)
+        return
 
-    not_any_failures = not any(notification.severity == Severity.ERROR for notification in notifications)
-    if not_any_failures and not silent:
-        text = "\nBIDS dataset was successfully created!\n\n"
-        console_notification = rich_click.style(text=text, fg="green")
+    if criticals:
+        text = (
+            "\nBIDS dataset was successfully created, but may not be valid!\n"
+            # TODO: "Please review the full notifications report at {run_config.log_file_path}\n\n"
+        )
+        console_notification = rich_click.style(text=text, fg="yellow")
         rich_click.echo(message=console_notification)
+        return
+
+    text = "\nBIDS dataset was successfully created!"
+    if notifications:
+        number_of_notifications = len(notifications)
+
+        text += (
+            f'\n{number_of_notifications} {_pluralize(n=number_of_notifications, phrase="suggestion")} for improvement '
+            f'{_pluralize(n=number_of_notifications, phrase="was", plural="were")} found during conversion.'
+            # TODO: " Please review the full notifications report at {run_config.log_file_path}\n"
+        )
+    text += "\n\n"
+    console_notification += rich_click.style(text=text, fg="green")
+    rich_click.echo(message=console_notification)
 
 
 # nwb2bids tutorial

src/nwb2bids/_tools/_pluralize.py

@@ -1,7 +1,7 @@
-def _pluralize(n: int, word: str, plural: str | None = None) -> str:
+def _pluralize(n: int, phrase: str, plural: str | None = None) -> str:
     if n == 1:
-        return word
+        return phrase
     elif plural is None:
-        return word + "s"
+        return phrase + "s"
     else:
         return plural

src/nwb2bids/bids_models/_bids_session_metadata.py

@@ -35,15 +35,15 @@ class BidsSessionMetadata(BaseMetadataContainerModel):
     @property
     def messages(self) -> list[InspectionResult]:
         messages = self.participant.messages.copy()
-        messages += self._internal_messages
+        messages += self._internal_messages.copy()
         if self.events is not None:
-            messages += self.events.messages
+            messages += self.events.messages.copy()
         if self.probe_table is not None:
-            messages += self.probe_table.messages
+            messages += self.probe_table.messages.copy()
         if self.channel_table is not None:
-            messages += self.channel_table.messages
+            messages += self.channel_table.messages.copy()
         if self.electrode_table is not None:
-            messages += self.electrode_table.messages
+            messages += self.electrode_table.messages.copy()
         messages.sort(key=lambda message: (-message.category.value, -message.severity.value, message.title))
         return messages
 

src/nwb2bids/bids_models/_probes.py

@@ -1,12 +1,13 @@
 import json
 import pathlib
+from typing import Any
 
 import pandas
 import pydantic
 import pynwb
 import typing_extensions
 
-from .._inspection._inspection_result import InspectionResult
+from .._inspection._inspection_result import Category, DataStandard, InspectionResult, Severity
 from ..bids_models._base_metadata_model import BaseMetadataContainerModel, BaseMetadataModel
 
 
@@ -20,6 +21,26 @@ class Probe(BaseMetadataModel):
 class ProbeTable(BaseMetadataContainerModel):
     probes: list[Probe]
 
+    def _check_fields(self) -> None:
+        # Check if values are specified
+        probes_missing_description = [probe for probe in self.probes if probe.description is None]
+        for probe_missing_description in probes_missing_description:
+            self.messages.append(
+                InspectionResult(
+                    title="Missing description",
+                    reason="A basic description of this field is recommended to improve contextual understanding.",
+                    solution="Add a description to the field.",
+                    field=f"nwbfile.devices.{probe_missing_description.probe_id}",
+                    source_file_paths=[],  # TODO: figure out better way of handling
+                    data_standards=[DataStandard.BIDS, DataStandard.NWB],
+                    category=Category.STYLE_SUGGESTION,
+                    severity=Severity.INFO,
+                )
+            )
+
+    def model_post_init(self, context: Any, /) -> None:
+        self._check_fields()
+
     @pydantic.computed_field
     @property
     def messages(self) -> list[InspectionResult]:

tests/conftest.py

@@ -4,6 +4,7 @@
 import pathlib
 import shutil
 import sys
+import uuid
 
 import py.path
 import pynwb
@@ -317,6 +318,29 @@ def problematic_nwbfile_path_3(testing_files_directory: pathlib.Path) -> pathlib
     return nwbfile_path
 
 
+@pytest.fixture(scope="session")
+def problematic_nwbfile_path_4(testing_files_directory: pathlib.Path) -> pathlib.Path:
+    """
+    A fourth NWB file with less problematic metadata corresponding only to low-level 'info' events.
+    """
+    nwbfile = pynwb.NWBFile(
+        identifier=str(uuid.uuid4()),
+        session_id="problematic4",
+        session_description="",
+        session_start_time=datetime.datetime.now().astimezone(),
+    )
+    device = pynwb.device.Device(name="DeviceWithoutDescription")
+    nwbfile.add_device(device)
+
+    problematic_subdirectory = testing_files_directory / "problematic"
+    problematic_subdirectory.mkdir(exist_ok=True)
+    nwbfile_path = problematic_subdirectory / "problematic4.nwb"
+    with pynwb.NWBHDF5IO(path=nwbfile_path, mode="w") as file_stream:
+        file_stream.write(nwbfile)
+
+    return nwbfile_path
+
+
 @pytest.fixture(scope="session")
 def problematic_nwbfile_path_missing_session_id(testing_files_directory: pathlib.Path) -> pathlib.Path:
     nwbfile = pynwb.testing.mock.file.mock_NWBFile(session_id=None)

tests/convert_nwb_dataset/test_cli_messages.py

@@ -4,12 +4,68 @@
 import subprocess
 
 
-def test_problematic_cli_messages(problematic_nwbfile_path_1: pathlib.Path, temporary_bids_directory: pathlib.Path):
+def test_problematic_cli_error_messages(
+    problematic_nwbfile_path_1: pathlib.Path, temporary_bids_directory: pathlib.Path
+):
     command = f"nwb2bids convert {problematic_nwbfile_path_1} -o {temporary_bids_directory}"
 
     result = subprocess.run(args=command, shell=True, capture_output=True)
     assert result.returncode == 0
-    assert b"4 suggestions for improvement were found during conversion." in result.stdout
+
+    expected_message = [
+        "",
+        "BIDS dataset was not successfully created!",
+        "Some errors were encountered during conversion.",
+        "The first 3 of 4 are shown below:",
+        "",
+        "",
+        "\t- Participant species is not a proper Latin binomial or NCBI Taxonomy id.",
+        "\t- The participant ID contains invalid characters. BIDS allows only the "
+        "plus sign to be used as a separator in the subject entity label. "
+        "Underscores, dashes, spaces, slashes, and other special characters "
+        "(including #) are expressly forbidden.",
+        "\t- Participant sex is not one of the allowed patterns by BIDS.",
+        "",
+        "",
+    ]
+    assert expected_message == result.stdout.decode(encoding="utf-8").splitlines()
+    assert result.stderr == b""
+
+
+def test_problematic_cli_critical_messages(
+    problematic_nwbfile_path_3: pathlib.Path, temporary_bids_directory: pathlib.Path
+):
+    command = f"nwb2bids convert {problematic_nwbfile_path_3} -o {temporary_bids_directory}"
+
+    result = subprocess.run(args=command, shell=True, capture_output=True)
+    assert result.returncode == 0
+
+    expected_message = ["", "BIDS dataset was successfully created, but may not be valid!", ""]
+    assert expected_message == result.stdout.decode(encoding="utf-8").splitlines()
+    assert result.stderr == b""
+
+
+def test_problematic_cli_info_messages(
+    problematic_nwbfile_path_4: pathlib.Path, temporary_bids_directory: pathlib.Path
+):
+    command = f"nwb2bids convert {problematic_nwbfile_path_4} -o {temporary_bids_directory}"
+
+    result = subprocess.run(args=command, shell=True, capture_output=True)
+    assert result.returncode == 0
+
+    expected_message = ["", "BIDS dataset was successfully created, but may not be valid!", ""]
+    assert expected_message == result.stdout.decode(encoding="utf-8").splitlines()
+    assert result.stderr == b""
+
+
+def test_problematic_cli_success(minimal_nwbfile_path: pathlib.Path, temporary_bids_directory: pathlib.Path):
+    command = f"nwb2bids convert {minimal_nwbfile_path} -o {temporary_bids_directory}"
+
+    result = subprocess.run(args=command, shell=True, capture_output=True)
+    assert result.returncode == 0
+
+    expected_message = ["", "BIDS dataset was successfully created!", "", ""]
+    assert expected_message == result.stdout.decode(encoding="utf-8").splitlines()
     assert result.stderr == b""