From 4c251eb0d564918171d4dd7977077b8ddc396f1b Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Thu, 7 Nov 2024 10:03:44 -0800 Subject: [PATCH 1/6] error when using colon for container name --- src/hdmf/container.py | 4 ++-- tests/unit/test_container.py | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/hdmf/container.py b/src/hdmf/container.py index 8f961936f..817eaa39e 100644 --- a/src/hdmf/container.py +++ b/src/hdmf/container.py @@ -303,8 +303,8 @@ def __new__(cls, *args, **kwargs): @docval({'name': 'name', 'type': str, 'doc': 'the name of this container'}) def __init__(self, **kwargs): name = getargs('name', kwargs) - if '/' in name: - raise ValueError("name '" + name + "' cannot contain '/'") + if ('/' in name or ':' in name) and not self._in_construct_mode: + raise ValueError(f"name '{name}' cannot contain '/' or ':'") self.__name = name self.__field_values = dict() self.__read_io = None diff --git a/tests/unit/test_container.py b/tests/unit/test_container.py index 35d8e480c..4fb31c6d0 100644 --- a/tests/unit/test_container.py +++ b/tests/unit/test_container.py @@ -179,6 +179,9 @@ def test_set_parent_overwrite_proxy(self): def test_slash_restriction(self): self.assertRaises(ValueError, Container, 'bad/name') + + def test_colon_restriction(self): + self.assertRaises(ValueError, Container, 'bad:name') def test_set_modified_parent(self): """Test that set modified properly sets parent modified From e4605bde2189e921f51dff843d976bf972929119 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 7 Nov 2024 18:10:32 +0000 Subject: [PATCH 2/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/unit/test_container.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_container.py b/tests/unit/test_container.py index 4fb31c6d0..4fe4c678d 100644 --- a/tests/unit/test_container.py +++ b/tests/unit/test_container.py @@ -179,7 +179,7 @@ def test_set_parent_overwrite_proxy(self): def test_slash_restriction(self): self.assertRaises(ValueError, Container, 'bad/name') - + def test_colon_restriction(self): self.assertRaises(ValueError, Container, 'bad:name') From 30dca970ca5f25914908c14df8ea0848a387a15d Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Thu, 7 Nov 2024 12:29:07 -0800 Subject: [PATCH 3/6] add test for construct mode --- src/hdmf/container.py | 2 +- tests/unit/test_container.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/hdmf/container.py b/src/hdmf/container.py index 817eaa39e..5f4ef8da8 100644 --- a/src/hdmf/container.py +++ b/src/hdmf/container.py @@ -304,7 +304,7 @@ def __new__(cls, *args, **kwargs): def __init__(self, **kwargs): name = getargs('name', kwargs) if ('/' in name or ':' in name) and not self._in_construct_mode: - raise ValueError(f"name '{name}' cannot contain '/' or ':'") + raise ValueError(f"name '{name}' cannot contain a '/' or ':'") self.__name = name self.__field_values = dict() self.__read_io = None diff --git a/tests/unit/test_container.py b/tests/unit/test_container.py index 4fe4c678d..3cfc72ec7 100644 --- a/tests/unit/test_container.py +++ b/tests/unit/test_container.py @@ -180,9 +180,17 @@ def test_set_parent_overwrite_proxy(self): def test_slash_restriction(self): self.assertRaises(ValueError, Container, 'bad/name') + # check no error raised in construct mode + child_obj = Container.__new__(Container, parent=None, object_id=None, in_construct_mode=True) + child_obj.__init__('bad/name') + def test_colon_restriction(self): self.assertRaises(ValueError, Container, 'bad:name') + # check no error raised in construct mode + child_obj = Container.__new__(Container, parent=None, object_id=None, in_construct_mode=True) + child_obj.__init__('bad:name') + def test_set_modified_parent(self): """Test that set modified properly sets parent modified """ From 01550f8026a7063a0f444520f1e2e5909833b451 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Thu, 7 Nov 2024 12:34:16 -0800 Subject: [PATCH 4/6] update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4f0fde80..ddeabc5e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Enhancements - Added support for expandable datasets of references for untyped and compound data types. @stephprince [#1188](https://github.com/hdmf-dev/hdmf/pull/1188) - Improved html representation of data in `Containers` @h-mayorquin [#1100](https://github.com/hdmf-dev/hdmf/pull/1100) +- Added error when using colon for `Container` name. @stephprince [#1202](https://github.com/hdmf-dev/hdmf/pull/1202) ### Bug fixes - Fixed inaccurate error message when validating reference data types. @stephprince [#1199](https://github.com/hdmf-dev/hdmf/pull/1199) From e3a36941a5119b4be5926513e75b4110b7dd1ae2 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Sat, 9 Nov 2024 12:25:41 -0800 Subject: [PATCH 5/6] Update CHANGELOG.md --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddeabc5e3..a69463ea6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,8 @@ ### Enhancements - Added support for expandable datasets of references for untyped and compound data types. @stephprince [#1188](https://github.com/hdmf-dev/hdmf/pull/1188) -- Improved html representation of data in `Containers` @h-mayorquin [#1100](https://github.com/hdmf-dev/hdmf/pull/1100) -- Added error when using colon for `Container` name. @stephprince [#1202](https://github.com/hdmf-dev/hdmf/pull/1202) +- Improved html representation of data in `Container` objects. @h-mayorquin [#1100](https://github.com/hdmf-dev/hdmf/pull/1100) +- Added error when using colon for `Container` name. A colon cannot be used as a group name when writing to Zarr on Windows. @stephprince [#1202](https://github.com/hdmf-dev/hdmf/pull/1202) ### Bug fixes - Fixed inaccurate error message when validating reference data types. @stephprince [#1199](https://github.com/hdmf-dev/hdmf/pull/1199) From 164e41961388bf742f9d9cff59ccceaf35253468 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Mon, 11 Nov 2024 09:59:58 -0800 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: Ryan Ly --- tests/unit/test_container.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_container.py b/tests/unit/test_container.py index 3cfc72ec7..c12247de7 100644 --- a/tests/unit/test_container.py +++ b/tests/unit/test_container.py @@ -181,14 +181,14 @@ def test_slash_restriction(self): self.assertRaises(ValueError, Container, 'bad/name') # check no error raised in construct mode - child_obj = Container.__new__(Container, parent=None, object_id=None, in_construct_mode=True) + child_obj = Container.__new__(Container, in_construct_mode=True) child_obj.__init__('bad/name') def test_colon_restriction(self): self.assertRaises(ValueError, Container, 'bad:name') # check no error raised in construct mode - child_obj = Container.__new__(Container, parent=None, object_id=None, in_construct_mode=True) + child_obj = Container.__new__(Container, in_construct_mode=True) child_obj.__init__('bad:name') def test_set_modified_parent(self):