From fd10fad5c0ed6c0f428090ae7bbf43d889842d76 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Mon, 2 Nov 2020 09:55:05 -0800 Subject: [PATCH 1/3] Fix overridden attributes not being removed from new attributes --- src/hdmf/spec/spec.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hdmf/spec/spec.py b/src/hdmf/spec/spec.py index 359a01736..1f023f8e7 100644 --- a/src/hdmf/spec/spec.py +++ b/src/hdmf/spec/spec.py @@ -374,7 +374,7 @@ def required(self): def resolve_spec(self, **kwargs): inc_spec = getargs('inc_spec', kwargs) for attribute in inc_spec.attributes: - self.__new_attributes.discard(attribute) + self.__new_attributes.discard(attribute.name) if attribute.name in self.__attributes: self.__overridden_attributes.add(attribute.name) continue From 115193680084f17a1c0e95b231f36ec203d4a525 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Mon, 2 Nov 2020 10:01:54 -0800 Subject: [PATCH 2/3] Change error to warning --- src/hdmf/container.py | 4 ++-- tests/unit/test_container.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hdmf/container.py b/src/hdmf/container.py index 285dd10d6..f3e2e906b 100644 --- a/src/hdmf/container.py +++ b/src/hdmf/container.py @@ -129,8 +129,8 @@ def __gather_fields(cls, name, bases, classdict): # check whether new fields spec already exists in base class for field_name in fields_dict: if field_name in base_fields: - raise ValueError("Field '%s' cannot be defined in %s. It already exists on base class %s." - % (field_name, cls.__name__, base_cls.__name__)) + warn("Field '%s' should not be defined in %s. It already exists on base class %s." + % (field_name, cls.__name__, base_cls.__name__)) # prepend field specs from base class to fields list of this class all_fields_conf[0:0] = base_cls.get_fields_conf() diff --git a/tests/unit/test_container.py b/tests/unit/test_container.py index a21281fd0..a3c48a201 100644 --- a/tests/unit/test_container.py +++ b/tests/unit/test_container.py @@ -372,9 +372,9 @@ def test_inheritance_override(self): class NamedFields(AbstractContainer): __fields__ = ({'name': 'field1'}, ) - msg = ("Field 'field1' cannot be defined in NamedFieldsChild. It already exists on base class " + msg = ("Field 'field1' should not be defined in NamedFieldsChild. It already exists on base class " "NamedFields.") - with self.assertRaisesWith(ValueError, msg): + with self.assertWarnsWith(UserWarning, msg): class NamedFieldsChild(NamedFields): __fields__ = ({'name': 'field1', 'settable': True}, ) From a53fa0e6d4ecdb6983cf145e6af42dc35b81012e Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Mon, 2 Nov 2020 10:07:17 -0800 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5a4a3f9d..31f848d09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ - Fix convert dtype when writing numpy array from `h5py.Dataset`. @rly (#427) - Fix inheritance when non-`AbstractContainer` is base class. @rly (#444) - Fix use of `hdmf.testing.assertContainerEqual(...)` for `Data` objects. @rly (#445) +- Fix bug in BaseStorageSpec.resolve_spec where attributes are not correctly removed from the set of new attributes. + @rly (#448) ## HDMF 2.2.0 (August 14, 2020)