Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raise best practice violation if ":" in container name #532

Open
wants to merge 15 commits into
base: dev
Choose a base branch
from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,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)
Expand Down
9 changes: 5 additions & 4 deletions docs/best_practices/general.rst
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,16 @@ It is recommended to use :wikipedia:`CamelCase <Camel_case>` 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 <faq_details/why_hdf5.html#why-hdf5>` for more details). The forward slash is used in `h5py` to specify a `Groups <https://schema-language.readthedocs.io/en/latest/description.html#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 <faq_details/why_hdf5.html#why-hdf5>` for more details) and/or Zarr files (which NWB allows as a backend). The forward slash is used in `h5py` to specify a `Groups <https://schema-language.readthedocs.io/en/latest/description.html#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`



Expand Down
2 changes: 2 additions & 0 deletions src/nwbinspector/checks/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
)
from ._general import (
check_description,
check_name_colons,
check_name_slashes,
)
from ._icephys import (
Expand Down Expand Up @@ -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",
Expand Down
9 changes: 9 additions & 0 deletions src/nwbinspector/checks/_general.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
"""
Expand Down
55 changes: 43 additions & 12 deletions tests/unit_tests/test_general.py
Original file line number Diff line number Diff line change
@@ -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():
Expand All @@ -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():
Expand Down
Loading