diff --git a/CHANGELOG.md b/CHANGELOG.md index ceb2b635..76ea8384 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ### Improvements * Added support for Numpy 2 and h5py 3.12, and pinned PyNWB to <3.0 temporarily. [#536](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/536) +* Added best practice around not using colons in object names. [#532](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/532) ### Fixes * Fixed issue where the description check failed if the description was a list. [#535](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/535) diff --git a/docs/best_practices/general.rst b/docs/best_practices/general.rst index f4ed6356..9c523047 100644 --- a/docs/best_practices/general.rst +++ b/docs/best_practices/general.rst @@ -34,15 +34,16 @@ It is recommended to use :wikipedia:`CamelCase ` when naming instanc .. _best_practice_name_slashes: +.. _best_practice_name_colons: -Do Not Use Slashes in Names -~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Do Not Use Slashes or Colons in Names +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Slash characters ``'/'`` and ``'\'`` should not be used in the ``name`` of an object, because they can be confusing to systems that parse HDF5 files (which NWB uses as the primary backend, see the :nwb-overview:`NWB FAQ ` for more details). The forward slash is used in `h5py` to specify a `Groups `_ hierarchy. +Slash characters ``'/'`` and ``'\'`` and colons ``':'`` should not be used in the ``name`` of an object, because they can be confusing to systems that parse HDF5 files (which NWB uses as the primary backend, see the :nwb-overview:`NWB FAQ ` for more details) and/or Zarr files (which NWB allows as a backend). The forward slash is used in `h5py` to specify a `Groups `_ hierarchy. For mathematical expressions, instead of including the special character in the name, please use an English equivalent instead. *E.g.*, instead of "Df/f" use "DfOverF". -Check function: :py:meth:`~nwbinspector.checks._general.check_name_slashes` +Check functions: :py:meth:`~nwbinspector.checks._general.check_name_slashes` and :py:meth:`~nwbinspector.checks._general.check_name_colons` diff --git a/src/nwbinspector/checks/__init__.py b/src/nwbinspector/checks/__init__.py index a24e6323..5c3d39a0 100644 --- a/src/nwbinspector/checks/__init__.py +++ b/src/nwbinspector/checks/__init__.py @@ -12,6 +12,7 @@ ) from ._general import ( check_description, + check_name_colons, check_name_slashes, ) from ._icephys import ( @@ -91,6 +92,7 @@ "check_spike_times_not_in_unobserved_interval", "check_description", "check_name_slashes", + "check_name_colons", "check_image_series_data_size", "check_image_series_external_file_relative", "check_image_series_external_file_valid", diff --git a/src/nwbinspector/checks/_general.py b/src/nwbinspector/checks/_general.py index 1eefd1a6..5aacbce6 100644 --- a/src/nwbinspector/checks/_general.py +++ b/src/nwbinspector/checks/_general.py @@ -16,6 +16,15 @@ def check_name_slashes(neurodata_object: object) -> Optional[InspectorMessage]: return None +@register_check(importance=Importance.BEST_PRACTICE_SUGGESTION, neurodata_type=None) +def check_name_colons(neurodata_object: object) -> Optional[InspectorMessage]: + """Check if an object name contains a colon.""" + if hasattr(neurodata_object, "name") and ":" in neurodata_object.name: + return InspectorMessage(message="Object name contains colons.") + + return None + + @register_check(importance=Importance.BEST_PRACTICE_SUGGESTION, neurodata_type=None) def check_description(neurodata_object: object) -> Optional[InspectorMessage]: """ diff --git a/tests/unit_tests/test_general.py b/tests/unit_tests/test_general.py index d386ad61..6231bd29 100644 --- a/tests/unit_tests/test_general.py +++ b/tests/unit_tests/test_general.py @@ -1,7 +1,7 @@ from hdmf.common import DynamicTable, DynamicTableRegion from nwbinspector import Importance, InspectorMessage -from nwbinspector.checks import check_description, check_name_slashes +from nwbinspector.checks import check_description, check_name_colons, check_name_slashes def test_check_name_slashes_pass(): @@ -10,17 +10,48 @@ def test_check_name_slashes_pass(): def test_check_name_slashes_fail(): - """HDMF/PyNWB forbid "/" in the object names. Might need an external file written in MATLAB to test that?""" - for x in ["\\"]: - table = DynamicTable(name=f"test{x}ing", description="") - assert check_name_slashes(neurodata_object=table) == InspectorMessage( - message="Object name contains slashes.", - importance=Importance.CRITICAL, - check_function_name="check_name_slashes", - object_type="DynamicTable", - object_name=f"test{x}ing", - location="/", - ) + # the latest version of HDMF/PyNWB forbids "/" in the object names when creating a new object + # so we use in_construct_mode=True to simulate an object that was read from a file + table = DynamicTable.__new__(DynamicTable, in_construct_mode=True) + table.__init__(name="test/ing", description="") + assert check_name_slashes(neurodata_object=table) == InspectorMessage( + message="Object name contains slashes.", + importance=Importance.CRITICAL, + check_function_name="check_name_slashes", + object_type="DynamicTable", + object_name="test/ing", + location="/", + ) + + table = DynamicTable(name="test\\ing", description="") + assert check_name_slashes(neurodata_object=table) == InspectorMessage( + message="Object name contains slashes.", + importance=Importance.CRITICAL, + check_function_name="check_name_slashes", + object_type="DynamicTable", + object_name="test\\ing", + location="/", + ) + + +def test_check_name_colons_pass(): + table = DynamicTable(name="test_name", description="") + assert check_name_colons(neurodata_object=table) is None + + +def test_check_name_colons_fail(): + # the latest version of HDMF/PyNWB forbids ":" in the object names when creating a new object + # so we use in_construct_mode=True to simulate an object that was read from a file + table = DynamicTable.__new__(DynamicTable, in_construct_mode=True) + table.__init__(name="test:ing", description="") + assert check_name_colons(neurodata_object=table) == InspectorMessage( + message="Object name contains colons.", + importance=Importance.BEST_PRACTICE_SUGGESTION, + check_function_name="check_name_colons", + object_type="DynamicTable", + object_name="test:ing", + location="/", + ) def test_check_description_pass():