From 25abcc1cf0b79bf9bff3371b6131eff4c90c9936 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Sat, 9 Nov 2024 16:24:12 -0800 Subject: [PATCH 01/14] Update general.rst --- docs/best_practices/general.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/best_practices/general.rst b/docs/best_practices/general.rst index f4ed6356b..7dd9b0da0 100644 --- a/docs/best_practices/general.rst +++ b/docs/best_practices/general.rst @@ -35,14 +35,14 @@ It is recommended to use :wikipedia:`CamelCase ` when naming instanc .. _best_practice_name_slashes: -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` From 1cf92edb5b15ee36c7f5be94e19f7cb949dae3d5 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Sat, 9 Nov 2024 16:26:09 -0800 Subject: [PATCH 02/14] Update _general.py --- src/nwbinspector/checks/_general.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/nwbinspector/checks/_general.py b/src/nwbinspector/checks/_general.py index f9b0b9563..283131a03 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 there has been added for the session.""" + 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]: """ From 523bc559419f64e1785e2654701d04b0417b5f25 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Sat, 9 Nov 2024 16:35:02 -0800 Subject: [PATCH 03/14] Update test_general.py --- tests/unit_tests/test_general.py | 53 +++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/tests/unit_tests/test_general.py b/tests/unit_tests/test_general.py index 10ebc6551..2bcd8fbbf 100644 --- a/tests/unit_tests/test_general.py +++ b/tests/unit_tests/test_general.py @@ -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 creating an object that was read from a file + table = DynamicTable.__new__(DynamicTable, in_construct_mode=True) + table.__init__(name=f"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=f"test/ing", + location="/", + ) + + table = DynamicTable(name=f"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=f"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_slashes_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 creating an object that was read from a file + table = DynamicTable.__new__(DynamicTable, in_construct_mode=True) + table.__init__(name=f"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=f"test:ing", + location="/", + ) def test_check_description_pass(): From 29b1fe8ed12941bf8b095d1cea4b8053f877c14a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 10 Nov 2024 00:36:43 +0000 Subject: [PATCH 04/14] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/unit_tests/test_general.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/unit_tests/test_general.py b/tests/unit_tests/test_general.py index 2bcd8fbbf..00aef4171 100644 --- a/tests/unit_tests/test_general.py +++ b/tests/unit_tests/test_general.py @@ -13,23 +13,23 @@ def test_check_name_slashes_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 creating an object that was read from a file table = DynamicTable.__new__(DynamicTable, in_construct_mode=True) - table.__init__(name=f"test/ing", description="") + 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=f"test/ing", + object_name="test/ing", location="/", ) - - table = DynamicTable(name=f"test\\ing", description="") + + 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=f"test\\ing", + object_name="test\\ing", location="/", ) @@ -43,13 +43,13 @@ def test_check_name_slashes_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 creating an object that was read from a file table = DynamicTable.__new__(DynamicTable, in_construct_mode=True) - table.__init__(name=f"test:ing", description="") + 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=f"test:ing", + object_name="test:ing", location="/", ) From 57abfc29e9c62d9894dbf581b8fe986677d49f14 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Sat, 9 Nov 2024 16:37:09 -0800 Subject: [PATCH 05/14] Update test_general.py --- tests/unit_tests/test_general.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/test_general.py b/tests/unit_tests/test_general.py index 00aef4171..1487d331b 100644 --- a/tests/unit_tests/test_general.py +++ b/tests/unit_tests/test_general.py @@ -11,7 +11,7 @@ def test_check_name_slashes_pass(): def test_check_name_slashes_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 creating an object that was read from a file + # 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( @@ -41,7 +41,7 @@ def test_check_name_colons_pass(): def test_check_name_slashes_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 creating an object that was read from a file + # 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( From 6423715e91b9e2b3d89ac69f90a5c2c172cbfddb Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Wed, 27 Nov 2024 17:21:35 -0800 Subject: [PATCH 06/14] Update general.rst --- docs/best_practices/general.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/best_practices/general.rst b/docs/best_practices/general.rst index 7dd9b0da0..9c5230475 100644 --- a/docs/best_practices/general.rst +++ b/docs/best_practices/general.rst @@ -34,6 +34,7 @@ It is recommended to use :wikipedia:`CamelCase ` when naming instanc .. _best_practice_name_slashes: +.. _best_practice_name_colons: Do Not Use Slashes or Colons in Names ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From 6d2ba6c34613c56a6525c98bce478bfbe803da1b Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Wed, 27 Nov 2024 17:22:41 -0800 Subject: [PATCH 07/14] Update src/nwbinspector/checks/_general.py Co-authored-by: Ben Dichter --- src/nwbinspector/checks/_general.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nwbinspector/checks/_general.py b/src/nwbinspector/checks/_general.py index 288090400..b137c11c8 100644 --- a/src/nwbinspector/checks/_general.py +++ b/src/nwbinspector/checks/_general.py @@ -18,7 +18,7 @@ def check_name_slashes(neurodata_object: object) -> Optional[InspectorMessage]: @register_check(importance=Importance.BEST_PRACTICE_SUGGESTION, neurodata_type=None) def check_name_colons(neurodata_object: object) -> Optional[InspectorMessage]: - """Check if there has been added for the session.""" + """Check if an object contains a colon.""" if hasattr(neurodata_object, "name") and ":" in neurodata_object.name: return InspectorMessage(message="Object name contains colons.") From 389fafbe1fe83e0dd8b0fac2045eb782a6144511 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Wed, 27 Nov 2024 17:23:40 -0800 Subject: [PATCH 08/14] Update test_general.py --- tests/unit_tests/test_general.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/test_general.py b/tests/unit_tests/test_general.py index 96237d8ed..0821f3a71 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_slashes, check_name_colons def test_check_name_slashes_pass(): @@ -39,7 +39,7 @@ def test_check_name_colons_pass(): assert check_name_colons(neurodata_object=table) is None -def test_check_name_slashes_fail(): +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) From 4764ce8f1dabeb1bd80411da69ef5dd820bb4f22 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 28 Nov 2024 01:23:49 +0000 Subject: [PATCH 09/14] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/unit_tests/test_general.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/test_general.py b/tests/unit_tests/test_general.py index 0821f3a71..6231bd299 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, check_name_colons +from nwbinspector.checks import check_description, check_name_colons, check_name_slashes def test_check_name_slashes_pass(): From 9085d835dba68eb52b9cd8b3562882278de4c657 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Wed, 27 Nov 2024 17:24:48 -0800 Subject: [PATCH 10/14] Fix import check_name_colons --- src/nwbinspector/checks/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nwbinspector/checks/__init__.py b/src/nwbinspector/checks/__init__.py index a24e6323d..b64ed35d9 100644 --- a/src/nwbinspector/checks/__init__.py +++ b/src/nwbinspector/checks/__init__.py @@ -13,6 +13,7 @@ from ._general import ( check_description, check_name_slashes, + check_name_colons, ) from ._icephys import ( check_intracellular_electrode_cell_id_exists, From 9e9a53cf87190c95b87a51d0f5d70598bb0e5144 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Wed, 27 Nov 2024 17:25:12 -0800 Subject: [PATCH 11/14] Fix import check_name_colons --- src/nwbinspector/checks/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nwbinspector/checks/__init__.py b/src/nwbinspector/checks/__init__.py index b64ed35d9..936ffb4a3 100644 --- a/src/nwbinspector/checks/__init__.py +++ b/src/nwbinspector/checks/__init__.py @@ -92,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", From 7a17cef931f5ee0eb0ffbfd98127161cc2549293 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 28 Nov 2024 01:25:38 +0000 Subject: [PATCH 12/14] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/nwbinspector/checks/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nwbinspector/checks/__init__.py b/src/nwbinspector/checks/__init__.py index 936ffb4a3..5c3d39a0e 100644 --- a/src/nwbinspector/checks/__init__.py +++ b/src/nwbinspector/checks/__init__.py @@ -12,8 +12,8 @@ ) from ._general import ( check_description, - check_name_slashes, check_name_colons, + check_name_slashes, ) from ._icephys import ( check_intracellular_electrode_cell_id_exists, From 396b5517e53e65287fd25ad56b3251e1449f7f6b Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Wed, 27 Nov 2024 17:30:15 -0800 Subject: [PATCH 13/14] Update src/nwbinspector/checks/_general.py Co-authored-by: Ben Dichter --- src/nwbinspector/checks/_general.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nwbinspector/checks/_general.py b/src/nwbinspector/checks/_general.py index b137c11c8..5aacbce69 100644 --- a/src/nwbinspector/checks/_general.py +++ b/src/nwbinspector/checks/_general.py @@ -18,7 +18,7 @@ def check_name_slashes(neurodata_object: object) -> Optional[InspectorMessage]: @register_check(importance=Importance.BEST_PRACTICE_SUGGESTION, neurodata_type=None) def check_name_colons(neurodata_object: object) -> Optional[InspectorMessage]: - """Check if an object contains a colon.""" + """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.") From 12d9561ea78ba48415ea3a21466725bbcef697c9 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Wed, 27 Nov 2024 18:03:59 -0800 Subject: [PATCH 14/14] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 839da9774..d8d588e66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)