diff --git a/CHANGELOG.md b/CHANGELOG.md index 443899e5f..a24ebd37e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ### New Checks * Added `check_spike_times_without_nans` to detect NaN values in Units spike times, which indicate conversion bugs or unclean array padding. [#689](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/689) * Added `check_subject_age_reference` to validate that `Subject.age__reference`, when present, is one of the supported values (`"birth"` or `"gestational"`). This catches invalid references in files written by tools that do not enforce the schema constraint. [#250](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/250) +* Added `check_meanings_table_includes_all_values` to detect values of a categorical column that have no entry in its `MeaningsTable`. [#718](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/718) +* Added `check_hed_annotations_valid`, `check_hed_lab_metadata_exists`, and `check_hed_value_vector_not_in_meanings_table` for HED annotations written with the `ndx-hed` extension. Each error found by the HED validator becomes its own inspector message, with errors that repeat down a column collapsed into one. Validating the annotations requires the extension, which is a new optional dependency installed with `pip install nwbinspector[hed]`. [#718](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/718) ### Improvements * Raised the minimum required PyNWB to `>=4.0` to track the latest PyNWB release. [#708](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/708) diff --git a/docs/api/checks.rst b/docs/api/checks.rst index 539970dfb..6c95d3178 100644 --- a/docs/api/checks.rst +++ b/docs/api/checks.rst @@ -51,3 +51,7 @@ ImageSeries Images ------ .. automodule:: nwbinspector.checks._images + +HED +--- +.. automodule:: nwbinspector.checks._hed diff --git a/docs/best_practices/best_practices_index.rst b/docs/best_practices/best_practices_index.rst index 7910fc13f..8c738fdb0 100644 --- a/docs/best_practices/best_practices_index.rst +++ b/docs/best_practices/best_practices_index.rst @@ -31,5 +31,6 @@ Authors: Oliver Ruebel, Andrew Tritt, Ryan Ly, Cody Baker and Ben Dichter ogen image_series images + hed simulated_data extensions diff --git a/docs/best_practices/hed.rst b/docs/best_practices/hed.rst new file mode 100644 index 000000000..88bbfa413 --- /dev/null +++ b/docs/best_practices/hed.rst @@ -0,0 +1,62 @@ +HED Annotations +=============== + +`HED `_ (Hierarchical Event Descriptors) is a controlled vocabulary for +annotating events and other tabular data. In NWB, HED annotations are stored with the +`ndx-hed `_ extension, which adds ``HedTags`` and ``HedValueVector`` +columns to any :ref:`hdmf-schema:sec-dynamictable` and a ``HedLabMetaData`` object that records the version of +the HED schema in use. Because the annotations are drawn from a formal vocabulary, they can be validated, and +the NWB Inspector reports each validation error as a separate message. + +Validating HED annotations requires the ``ndx-hed`` package, which is not installed by default. Install it with +``pip install nwbinspector[hed]``. Without it, the NWB Inspector will read files that contain HED annotations +but will not validate them. + +.. _best_practice_hed_lab_metadata: + +Declare the HED Schema Version +------------------------------ + +A file that contains HED annotations should also contain a ``HedLabMetaData`` object giving the version of the +HED schema that the annotations were written against. The vocabulary changes between versions, so without the +version the annotations cannot be interpreted or validated: + +.. code-block:: python + + from ndx_hed import HedLabMetaData + + nwbfile.add_lab_meta_data(HedLabMetaData(hed_schema_version="8.4.0")) + +Check function: :py:meth:`~nwbinspector.checks._hed.check_hed_lab_metadata_exists` + +.. _best_practice_hed_annotations: + +Use Valid HED Annotations +------------------------- + +Every HED annotation in the file should validate against the declared HED schema. Common errors are tags that +are not in the schema, tags that are misspelled, and value templates that expand into invalid tags once the +value from the column is substituted for the ``#`` placeholder. The NWB Inspector reports the errors found by +the validator that ``ndx-hed`` provides, one message per error, with an error that repeats down a column +collapsed into a single message. + +The inspector validates each annotated column of each table on its own, which is what the annotation of that +column means in isolation. It does not perform the assembled validation that ``ndx-hed`` offers, which combines +all the annotations of a row into one HED string and validates the table as a timeline. That form of validation +requires reading the whole table into memory, which the inspector avoids so that it can run on large files and +on files read over a network. Run ``HedNWBValidator.validate_file`` from ``ndx-hed`` directly to get it. + +Check function: :py:meth:`~nwbinspector.checks._hed.check_hed_annotations_valid` + +.. _best_practice_hed_value_vector_meanings_table: + +Annotate a MeaningsTable with HedTags, Not HedValueVector +--------------------------------------------------------- + +A ``MeaningsTable`` assigns a meaning to each individual value of a categorical column, so its HED annotations +are complete HED strings stored in a ``HedTags`` column, one annotation per value. A ``HedValueVector`` is the +opposite construct: a single template annotation with a ``#`` placeholder that each value of a column is +substituted into. A template has no role inside a ``MeaningsTable``, and the ``ndx-hed`` validator rejects +files that place one there. + +Check function: :py:meth:`~nwbinspector.checks._hed.check_hed_value_vector_not_in_meanings_table` diff --git a/docs/best_practices/tables.rst b/docs/best_practices/tables.rst index fc27f370a..610517a5b 100644 --- a/docs/best_practices/tables.rst +++ b/docs/best_practices/tables.rst @@ -142,3 +142,17 @@ descendants of :ref:`hdmf-schema:sec-dynamictable` such as :ref:`nwb-schema:sec- ``ElectrodesTable``. In PyNWB, rows of :ref:`hdmf-schema:sec-dynamictable` increment as you add rows, so this variable is unique by default. If you would like to make values of ``id`` non-unique, a better solution would be to store these values as a custom column and use the default ``id`` values. + +.. _best_practice_meanings_table_includes_all_values: + +Complete MeaningsTables +~~~~~~~~~~~~~~~~~~~~~~~ + +A ``MeaningsTable`` describes the meaning of each value that a categorical column can take. Every value that +occurs in the annotated column should have an entry in the ``value`` column of its ``MeaningsTable``; a value +without an entry has no recorded meaning, and tools that consume the meanings (for example, HED annotations of +the categories) cannot process it. The reverse is allowed: the ``MeaningsTable`` may contain entries for values +that never occur in the column, such as a condition that was planned but not run. The values ``None``, ``""``, +and ``"n/a"`` mark missing data and do not need an entry. + +Check function: :py:meth:`~nwbinspector.checks._tables.check_meanings_table_includes_all_values` diff --git a/pyproject.toml b/pyproject.toml index 63350a016..21076b0bd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,6 +54,9 @@ dandi = [ zarr = [ "hdmf-zarr", ] +hed = [ + "ndx-hed>0.2.0", # 0.2.0 is the last release that predates the ndx-hed support for pynwb>=4.0 +] [tool.hatch.build.targets.sdist] exclude = [ diff --git a/src/nwbinspector/checks/__init__.py b/src/nwbinspector/checks/__init__.py index 8b5beac0d..0d8351d9f 100644 --- a/src/nwbinspector/checks/__init__.py +++ b/src/nwbinspector/checks/__init__.py @@ -25,6 +25,11 @@ check_name_colons, check_name_slashes, ) +from ._hed import ( + check_hed_annotations_valid, + check_hed_lab_metadata_exists, + check_hed_value_vector_not_in_meanings_table, +) from ._icephys import ( check_intracellular_electrode_cell_id_exists, check_intracellular_electrode_location_allen_ccf, @@ -87,6 +92,7 @@ check_dynamic_table_region_data_validity, check_empty_table, check_ids_unique, + check_meanings_table_includes_all_values, check_single_row, check_table_time_columns_are_not_negative, check_table_values_for_dict, @@ -161,6 +167,7 @@ "check_roi_response_series_link_to_plane_segmentation", "check_single_row", "check_ids_unique", + "check_meanings_table_includes_all_values", "check_empty_table", "check_col_not_nan", "check_column_binary_capability", @@ -200,4 +207,7 @@ "check_electrodes_location_allen_ccf", "check_intracellular_electrode_location_allen_ccf", "check_units_table_has_spikes", + "check_hed_lab_metadata_exists", + "check_hed_annotations_valid", + "check_hed_value_vector_not_in_meanings_table", ] diff --git a/src/nwbinspector/checks/_hed.py b/src/nwbinspector/checks/_hed.py new file mode 100644 index 000000000..b696884b9 --- /dev/null +++ b/src/nwbinspector/checks/_hed.py @@ -0,0 +1,200 @@ +"""Check functions for HED (Hierarchical Event Descriptor) annotations added by the ``ndx-hed`` extension.""" + +from collections import defaultdict +from typing import Any, Iterable, Optional + +from hdmf.common import DynamicTable, MeaningsTable +from pynwb import NWBFile + +from .._registration import Importance, InspectorMessage, register_check + +# The ndx-hed package is an optional dependency, installed with `pip install nwbinspector[hed]`. +# ImportError rather than ModuleNotFoundError is caught because releases of ndx-hed that predate its +# support for pynwb>=4.0 import packages that are not compatible with the rest of this environment. +try: + from hed.errors import HedFileError + from ndx_hed import HedLabMetaData + from ndx_hed.utils.hed_nwb_validator import HedNWBValidator + + _HAS_NDX_HED = True +except ImportError: + _HAS_NDX_HED = False + +# Names of the neurodata types that the ndx-hed extension uses to store HED annotations in a table column. +# These are compared by name so that the annotations can be found even when the ndx-hed package is not +# installed, in which case PyNWB builds the classes on the fly from the specification cached in the file. +_HED_COLUMN_TYPE_NAMES = ("HedTags", "HedValueVector") + +# The HedLabMetaData object always has this fixed name +_HED_LAB_META_DATA_NAME = "hed_schema" + +_MAXIMUM_NUMBER_OF_LISTED_COLUMNS = 3 + + +def _get_hed_columns_of_table(table: DynamicTable) -> list: + """Return the columns of a table that hold HED annotations.""" + return [column for column in table.columns if type(column).__name__ in _HED_COLUMN_TYPE_NAMES] + + +def _get_hed_columns_of_file(nwbfile: NWBFile) -> list: + """Return every column of every table in the file that holds HED annotations.""" + return [ + neurodata_object + for neurodata_object in nwbfile.objects.values() + if type(neurodata_object).__name__ in _HED_COLUMN_TYPE_NAMES + ] + + +def _describe_column(column: Any) -> str: + """Describe a HED column by its name and the name of the table that contains it.""" + if getattr(column, "parent", None) is None: + return f"'{column.name}'" + + return f"'{column.name}' of table '{column.parent.name}'" + + +def _get_hed_lab_meta_data(nwbfile: Optional[NWBFile]) -> Optional["HedLabMetaData"]: + """Return the object that declares the HED schema version of the file, if there is one.""" + if nwbfile is None: + return None + + hed_lab_meta_data = nwbfile.lab_meta_data.get(_HED_LAB_META_DATA_NAME, None) + + return hed_lab_meta_data if isinstance(hed_lab_meta_data, HedLabMetaData) else None + + +def _group_key(issue: dict) -> tuple: + """ + Identify the issues that should be reported together as a single message. + + A single mistake that is repeated down a column, such as the same misspelled tag on every row, is + reported by the HED validator once per row. The offending tag is part of the key so that different + mistakes in the same column are still reported separately. + """ + source_tag = issue.get("source_tag", None) + + return (issue.get("ec_column", None), issue.get("code", None), str(source_tag) if source_tag is not None else None) + + +def _format_issue_message(issues: list[dict[str, Any]]) -> str: + """Compose the message for a group of issues that share a column, an error code, and a tag.""" + first_issue = issues[0] + column_name = first_issue.get("ec_column", None) + row_index = first_issue.get("ec_row", None) + error_code = first_issue.get("code", None) + + source = f" in column '{column_name}'" if column_name is not None else "" + if row_index is not None: + source += f", row {row_index}" + if error_code is not None: + source += f" ({error_code})" + + reported_message = " ".join(str(first_issue.get("message", "no message was reported")).split()) + if not reported_message.endswith("."): + reported_message += "." + + message = f"HED validation error{source}: {reported_message}" + if len(issues) > 1: + message += f" This error occurs on {len(issues)} rows; only the first is reported." + + return message + + +@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=NWBFile) +def check_hed_lab_metadata_exists(nwbfile: NWBFile) -> Optional[InspectorMessage]: + """ + Check that a file containing HED annotations also declares the HED schema version they use. + + Best Practice: :ref:`best_practice_hed_lab_metadata` + """ + hed_columns = _get_hed_columns_of_file(nwbfile=nwbfile) + if not hed_columns: + return None + if nwbfile.lab_meta_data.get(_HED_LAB_META_DATA_NAME, None) is not None: + return None + + # Sorted so that the message does not depend on the order in which the objects happen to be traversed + described_columns = sorted(_describe_column(column=column) for column in hed_columns) + listed_columns = described_columns[:_MAXIMUM_NUMBER_OF_LISTED_COLUMNS] + number_of_unlisted_columns = len(described_columns) - len(listed_columns) + columns_text = ", ".join(listed_columns) + if number_of_unlisted_columns > 0: + columns_text += f" (and {number_of_unlisted_columns} more)" + + return InspectorMessage( + message=( + f"This file contains HED annotations in column {columns_text}, but it does not contain a " + "HedLabMetaData object declaring the version of the HED schema that those annotations use. " + "Without it the annotations cannot be interpreted or validated. Add one with, for example, " + "`nwbfile.add_lab_meta_data(HedLabMetaData(hed_schema_version='8.4.0'))`." + ) + ) + + +@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=DynamicTable) +def check_hed_annotations_valid(table: DynamicTable) -> Optional[Iterable[InspectorMessage]]: + """ + Check that the HED annotations of a table are valid against the HED schema declared by the file. + + Every error reported by the HED validator becomes its own message, except that an error repeated + down a column is reported once with the number of rows it affects. + + The columns of the table are validated one at a time, which is what the ``HedTags`` and + ``HedValueVector`` annotations mean on their own. Validation of the annotation of a row as a whole, + which combines the columns of that row and requires reading the entire table into memory, is not + performed here. + + This check requires the ``ndx-hed`` package, which is installed with `pip install nwbinspector[hed]`. + Without it, the HED annotations of a file are not validated. + + Best Practice: :ref:`best_practice_hed_annotations` + """ + if not _HAS_NDX_HED: + return None + if not _get_hed_columns_of_table(table=table): + return None + + hed_lab_meta_data = _get_hed_lab_meta_data(nwbfile=table.get_ancestor("NWBFile")) + if hed_lab_meta_data is None: + return None # A file without the HED schema version is reported by check_hed_lab_metadata_exists + + try: + issues = HedNWBValidator(hed_lab_meta_data).validate_table(table) + except (HedFileError, ValueError) as exception: + yield InspectorMessage(message=f"The HED annotations of this table could not be validated: {exception}") + return None + + grouped_issues: dict = defaultdict(list) + for issue in issues: + grouped_issues[_group_key(issue=issue)].append(issue) + + for issues_in_group in grouped_issues.values(): + yield InspectorMessage(message=_format_issue_message(issues=issues_in_group)) + + return None + + +@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=MeaningsTable) +def check_hed_value_vector_not_in_meanings_table(meanings_table: MeaningsTable) -> Optional[Iterable[InspectorMessage]]: + """ + Check that a MeaningsTable does not store its HED annotations in a HedValueVector column. + + A MeaningsTable assigns a meaning to each individual value of a categorical column, so its HED + annotations are complete HED strings in a ``HedTags`` column. A ``HedValueVector`` is a template + with a ``#`` placeholder that a value from the column is substituted into, which has no role in a + MeaningsTable. + + Best Practice: :ref:`best_practice_hed_value_vector_meanings_table` + """ + for column in meanings_table.columns: + if type(column).__name__ == "HedValueVector": + yield InspectorMessage( + message=( + f"Column '{column.name}' of MeaningsTable '{meanings_table.name}' is a HedValueVector, " + "which is not allowed in a MeaningsTable. A MeaningsTable assigns a HED annotation to " + "each individual value of a categorical column, so its annotations must be complete " + "HED strings stored in a HedTags column." + ) + ) + + return None diff --git a/src/nwbinspector/checks/_tables.py b/src/nwbinspector/checks/_tables.py index e54b74e52..5b8d4bf15 100644 --- a/src/nwbinspector/checks/_tables.py +++ b/src/nwbinspector/checks/_tables.py @@ -4,7 +4,7 @@ from typing import Iterable, Optional import numpy as np -from hdmf.common import DynamicTable, DynamicTableRegion, VectorIndex +from hdmf.common import DynamicTable, DynamicTableRegion, MeaningsTable, VectorIndex from pynwb.file import TimeIntervals, Units from .._registration import Importance, InspectorMessage, register_check @@ -399,3 +399,55 @@ def check_time_intervals_duration( ) ) return None + + +def _normalize_categorical_value(value: object) -> str: + """Normalize a value of a categorical column for comparison against the MeaningsTable entries.""" + if isinstance(value, bytes): + value = value.decode("utf-8") + + return str(value) + + +@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=MeaningsTable) +def check_meanings_table_includes_all_values( + meanings_table: MeaningsTable, maximum_number_of_listed_values: int = 5 +) -> Optional[InspectorMessage]: + """ + Check that every value of the column annotated by a MeaningsTable has an entry in the table. + + The values ``None``, ``""``, and ``"n/a"`` mark missing data and do not need an entry. The reverse + is allowed: a MeaningsTable may contain entries for values that never occur in the column. + + Best Practice: :ref:`best_practice_meanings_table_includes_all_values` + """ + target = meanings_table.target + if target is None or "value" not in meanings_table.colnames: + return None + + known_values = {_normalize_categorical_value(value=value) for value in meanings_table["value"].data[:]} + + missing_values: list[str] = [] # in order of first appearance, so that the message is deterministic + for value in target.data[:]: + if value is None or (isinstance(value, Real) and np.isnan(value)): + continue + normalized_value = _normalize_categorical_value(value=value) + if normalized_value in ("", "n/a") or normalized_value in known_values or normalized_value in missing_values: + continue + missing_values.append(normalized_value) + + if not missing_values: + return None + + listed_values = ", ".join(f"'{value}'" for value in missing_values[:maximum_number_of_listed_values]) + number_of_unlisted_values = len(missing_values) - min(len(missing_values), maximum_number_of_listed_values) + if number_of_unlisted_values > 0: + listed_values += f" (and {number_of_unlisted_values} more)" + + return InspectorMessage( + message=( + f"Column '{target.name}' contains values that have no entry in its MeaningsTable " + f"'{meanings_table.name}': {listed_values}. Every value of the annotated column should have " + "an entry in the 'value' column of the MeaningsTable describing its meaning." + ) + ) diff --git a/tests/unit_tests/test_hed.py b/tests/unit_tests/test_hed.py new file mode 100644 index 000000000..6ce34e277 --- /dev/null +++ b/tests/unit_tests/test_hed.py @@ -0,0 +1,246 @@ +"""Tests for the HED checks. + +These require the ``ndx-hed`` package, which is an optional dependency of the NWB Inspector +(`pip install nwbinspector[hed]`). The whole module is skipped when it is not installed. +""" + +from datetime import datetime, timezone + +import pytest +from hdmf.common import DynamicTable, MeaningsTable, VectorData +from pynwb import NWBFile + +from nwbinspector import Importance, InspectorMessage, default_check_registry +from nwbinspector.checks import ( + check_hed_annotations_valid, + check_hed_lab_metadata_exists, + check_hed_value_vector_not_in_meanings_table, +) + +pytest.importorskip("ndx_hed", reason="The 'ndx-hed' package is required for the HED checks.") + +from ndx_hed import HedLabMetaData, HedTags, HedValueVector # noqa: E402 + +HED_SCHEMA_VERSION = "8.4.0" + + +def _make_nwbfile(add_hed_lab_meta_data: bool = True) -> NWBFile: + nwbfile = NWBFile( + session_description="Testing HED annotations.", + identifier="hed_test", + session_start_time=datetime(2020, 1, 1, tzinfo=timezone.utc), + ) + if add_hed_lab_meta_data: + nwbfile.add_lab_meta_data(HedLabMetaData(hed_schema_version=HED_SCHEMA_VERSION)) + + return nwbfile + + +def _add_hed_tags_table(nwbfile: NWBFile, tags: list, name: str = "events") -> DynamicTable: + table = DynamicTable( + name=name, + description="Events annotated with HED.", + columns=[ + VectorData(name="event_time", description="Event times.", data=list(range(len(tags)))), + HedTags(data=tags), + ], + ) + nwbfile.add_acquisition(table) + + return table + + +def test_check_hed_lab_metadata_exists_pass(): + nwbfile = _make_nwbfile() + _add_hed_tags_table(nwbfile=nwbfile, tags=["Sensory-event", "Agent-action"]) + + assert check_hed_lab_metadata_exists(nwbfile=nwbfile) is None + + +def test_check_hed_lab_metadata_exists_fail(): + nwbfile = _make_nwbfile(add_hed_lab_meta_data=False) + _add_hed_tags_table(nwbfile=nwbfile, tags=["Sensory-event", "Agent-action"]) + + assert check_hed_lab_metadata_exists(nwbfile=nwbfile) == InspectorMessage( + message=( + "This file contains HED annotations in column 'HED' of table 'events', but it does not contain a " + "HedLabMetaData object declaring the version of the HED schema that those annotations use. " + "Without it the annotations cannot be interpreted or validated. Add one with, for example, " + "`nwbfile.add_lab_meta_data(HedLabMetaData(hed_schema_version='8.4.0'))`." + ), + importance=Importance.BEST_PRACTICE_VIOLATION, + check_function_name="check_hed_lab_metadata_exists", + object_type="NWBFile", + object_name="root", + location="/", + ) + + +def test_check_hed_lab_metadata_exists_lists_only_a_few_columns(): + nwbfile = _make_nwbfile(add_hed_lab_meta_data=False) + for table_index in range(5): + _add_hed_tags_table(nwbfile=nwbfile, tags=["Sensory-event"], name=f"events_{table_index}") + + message = check_hed_lab_metadata_exists(nwbfile=nwbfile).message + assert "'HED' of table 'events_0'" in message + assert "'HED' of table 'events_2'" in message + assert "'HED' of table 'events_3'" not in message + assert "(and 2 more)" in message + + +def test_check_hed_lab_metadata_exists_skipped_without_hed_columns(): + nwbfile = _make_nwbfile(add_hed_lab_meta_data=False) + nwbfile.add_acquisition( + DynamicTable( + name="events", + description="Events without HED annotations.", + columns=[VectorData(name="event_time", description="Event times.", data=[1.0, 2.0])], + ) + ) + + assert check_hed_lab_metadata_exists(nwbfile=nwbfile) is None + + +def test_check_hed_annotations_valid_pass(): + nwbfile = _make_nwbfile() + table = _add_hed_tags_table(nwbfile=nwbfile, tags=["Sensory-event, Visual-presentation", "Agent-action"]) + + assert check_hed_annotations_valid(table=table) is None + + +def test_check_hed_annotations_valid_skipped_without_hed_columns(): + nwbfile = _make_nwbfile() + table = DynamicTable( + name="events", + description="Events without HED annotations.", + columns=[VectorData(name="event_time", description="Event times.", data=[1.0, 2.0])], + ) + nwbfile.add_acquisition(table) + + assert check_hed_annotations_valid(table=table) is None + + +def test_check_hed_annotations_valid_skipped_without_lab_metadata(): + """A file without a HedLabMetaData cannot be validated, which check_hed_lab_metadata_exists reports.""" + nwbfile = _make_nwbfile(add_hed_lab_meta_data=False) + table = _add_hed_tags_table(nwbfile=nwbfile, tags=["NonExistentEvent"]) + + assert check_hed_annotations_valid(table=table) is None + + +def test_check_hed_annotations_valid_fail(): + nwbfile = _make_nwbfile() + table = _add_hed_tags_table(nwbfile=nwbfile, tags=["Sensory-event", "NonExistentEvent", "AnotherBadTag"]) + + messages = list(check_hed_annotations_valid(table=table)) + + assert len(messages) == 2 + assert messages[0] == InspectorMessage( + message=( + "HED validation error in column 'HED', row 1 (TAG_INVALID): " + "'NonExistentEvent' in NonExistentEvent is not a valid base HED tag." + ), + importance=Importance.BEST_PRACTICE_VIOLATION, + check_function_name="check_hed_annotations_valid", + object_type="DynamicTable", + object_name="events", + location=None, + ) + assert "'AnotherBadTag'" in messages[1].message + assert "row 2" in messages[1].message + + +def test_check_hed_annotations_valid_reports_the_row_of_the_offending_tag(): + nwbfile = _make_nwbfile() + tags = ["Sensory-event"] * 7 + ["NonExistentEvent"] + table = _add_hed_tags_table(nwbfile=nwbfile, tags=tags) + + messages = list(check_hed_annotations_valid(table=table)) + + assert len(messages) == 1 + assert f"row {tags.index('NonExistentEvent')} " in messages[0].message + + +def test_check_hed_annotations_valid_collapses_repeats_of_the_same_error(): + """The same mistake repeated down a column is reported by the validator once per row, but once here.""" + nwbfile = _make_nwbfile() + table = _add_hed_tags_table(nwbfile=nwbfile, tags=["NonExistentEvent"] * 4) + + messages = list(check_hed_annotations_valid(table=table)) + + assert len(messages) == 1 + assert "This error occurs on 4 rows; only the first is reported." in messages[0].message + + +def test_check_hed_annotations_valid_of_a_value_vector(): + nwbfile = _make_nwbfile() + table = DynamicTable( + name="trials", + description="Trials with an invalid HED value template.", + columns=[ + VectorData(name="trial_id", description="Trial IDs.", data=[1, 2, 3]), + HedValueVector( + name="reaction_time", + description="Response times.", + data=[500, 700, 300], + hed="InvalidValueTag/#", + ), + ], + ) + nwbfile.add_acquisition(table) + + messages = list(check_hed_annotations_valid(table=table)) + + assert len(messages) == 1 + assert "in column 'reaction_time'" in messages[0].message + assert "'InvalidValueTag'" in messages[0].message + + +def _make_meanings_table(annotation_column) -> MeaningsTable: + target = VectorData(name="response", description="Response codes.", data=["go", "stop"]) + meanings_table = MeaningsTable(target=target, description="Meanings of response codes.") + meanings_table.add_row(value="go", meaning="A go trial.") + meanings_table.add_row(value="stop", meaning="A stop trial.") + meanings_table.add_column( + name=annotation_column.pop("name"), description="HED annotations of the values.", **annotation_column + ) + + return meanings_table + + +def test_check_hed_value_vector_not_in_meanings_table_pass(): + meanings_table = _make_meanings_table( + annotation_column=dict(name="HED", col_cls=HedTags, data=["Sensory-event", "Agent-action"]) + ) + + assert check_hed_value_vector_not_in_meanings_table(meanings_table=meanings_table) is None + + +def test_check_hed_value_vector_not_in_meanings_table_fail(): + meanings_table = _make_meanings_table( + annotation_column=dict(name="template", col_cls=HedValueVector, data=[1, 2], hed="Duration/# s") + ) + + messages = list(check_hed_value_vector_not_in_meanings_table(meanings_table=meanings_table)) + + assert messages == [ + InspectorMessage( + message=( + "Column 'template' of MeaningsTable 'response_meanings' is a HedValueVector, " + "which is not allowed in a MeaningsTable. A MeaningsTable assigns a HED annotation to " + "each individual value of a categorical column, so its annotations must be complete " + "HED strings stored in a HedTags column." + ), + importance=Importance.BEST_PRACTICE_VIOLATION, + check_function_name="check_hed_value_vector_not_in_meanings_table", + object_type="MeaningsTable", + object_name="response_meanings", + location="/", + ) + ] + + +def test_hed_checks_are_registered(): + assert "check_hed_lab_metadata_exists" in default_check_registry + assert "check_hed_annotations_valid" in default_check_registry + assert "check_hed_value_vector_not_in_meanings_table" in default_check_registry diff --git a/tests/unit_tests/test_tables.py b/tests/unit_tests/test_tables.py index fdae087d7..11e6491f8 100644 --- a/tests/unit_tests/test_tables.py +++ b/tests/unit_tests/test_tables.py @@ -3,7 +3,7 @@ from unittest import TestCase import numpy as np -from hdmf.common import DynamicTable, DynamicTableRegion +from hdmf.common import DynamicTable, DynamicTableRegion, MeaningsTable, VectorData from numpy.lib import NumpyVersion from pynwb.file import Device, ElectrodeGroup, ElectrodesTable, TimeIntervals, Units @@ -14,6 +14,7 @@ check_dynamic_table_region_data_validity, check_empty_table, check_ids_unique, + check_meanings_table_includes_all_values, check_single_row, check_table_time_columns_are_not_negative, check_table_values_for_dict, @@ -651,3 +652,59 @@ def test_check_time_intervals_duration_pass_without_additional_time_columns(): table.add_row(start_time=15.0, stop_time=25.0, custom_time=20.0) assert check_time_intervals_duration(table) is None + + +def _make_meanings_table(column_values: list, mapped_values: list) -> MeaningsTable: + target = VectorData(name="response", description="Response codes.", data=column_values) + meanings_table = MeaningsTable(target=target, description="Meanings of response codes.") + for value in mapped_values: + meanings_table.add_row(value=value, meaning=f"The meaning of {value}.") + + return meanings_table + + +def test_check_meanings_table_includes_all_values_pass(): + meanings_table = _make_meanings_table(column_values=["go", "stop", "go"], mapped_values=["go", "stop"]) + + assert check_meanings_table_includes_all_values(meanings_table=meanings_table) is None + + +def test_check_meanings_table_includes_all_values_extra_entries_are_allowed(): + """The reverse direction is not an error: entries for values that never occur in the column.""" + meanings_table = _make_meanings_table(column_values=["go", "go"], mapped_values=["go", "stop", "pause"]) + + assert check_meanings_table_includes_all_values(meanings_table=meanings_table) is None + + +def test_check_meanings_table_includes_all_values_missing_data_needs_no_entry(): + meanings_table = _make_meanings_table(column_values=["go", "n/a", "", None], mapped_values=["go"]) + + assert check_meanings_table_includes_all_values(meanings_table=meanings_table) is None + + +def test_check_meanings_table_includes_all_values_fail(): + meanings_table = _make_meanings_table(column_values=["go", "stop", "pause"], mapped_values=["go"]) + + assert check_meanings_table_includes_all_values(meanings_table=meanings_table) == InspectorMessage( + message=( + "Column 'response' contains values that have no entry in its MeaningsTable " + "'response_meanings': 'stop', 'pause'. Every value of the annotated column should have " + "an entry in the 'value' column of the MeaningsTable describing its meaning." + ), + importance=Importance.BEST_PRACTICE_VIOLATION, + check_function_name="check_meanings_table_includes_all_values", + object_type="MeaningsTable", + object_name="response_meanings", + location="/", + ) + + +def test_check_meanings_table_includes_all_values_lists_only_a_few_values(): + column_values = [f"code_{index}" for index in range(8)] + meanings_table = _make_meanings_table(column_values=column_values, mapped_values=["code_0"]) + + message = check_meanings_table_includes_all_values(meanings_table=meanings_table).message + assert "'code_1'" in message + assert "'code_5'" in message + assert "'code_6'" not in message + assert "(and 2 more)" in message