Skip to content

Commit

Permalink
Refactor DTR warning
Browse files Browse the repository at this point in the history
  • Loading branch information
rly committed Jul 25, 2023
1 parent ff30e12 commit 0dd9dd0
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 7 deletions.
11 changes: 11 additions & 0 deletions src/hdmf/common/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -1421,6 +1421,17 @@ def __repr__(self):
id(self.table))
return template

def _validate_on_set_parent(self):
# when this DynamicTableRegion is added to a parent, check whether one of the ancestors is
# also an ancestor of the linked-to table.
table_ancestor_ids = [id(x) for x in self.table.get_ancestors()]
self_ancestor_ids = [id(x) for x in self.get_ancestors()]
if set(table_ancestor_ids).isdisjoint(self_ancestor_ids):
msg = (f"The table for DynamicTableRegion '{self.name}' does not share an ancestor with the "
"DynamicTableRegion.")
warn(msg)
return super()._validate_on_set_parent()


def _uint_precision(elements):
""" Calculate the uint precision needed to encode a set of elements """
Expand Down
25 changes: 19 additions & 6 deletions src/hdmf/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,15 @@ def get_ancestor(self, **kwargs):
p = p.parent
return None

@docval()
def get_ancestors(self, **kwargs):
p = self.parent
ret = []
while p is not None:
ret.append(p)
p = p.parent
return tuple(ret)

@property
def fields(self):
'''
Expand Down Expand Up @@ -411,12 +420,8 @@ def parent(self, parent_container):
parent_container.__children.append(self)
parent_container.set_modified()
for child in self.children:
if type(child).__name__ == "DynamicTableRegion":
if child.table.parent is None:
msg = "The table for this DynamicTableRegion has not been added to the parent."
warn(msg)
else:
continue
# used by hdmf.common.table.DynamicTableRegion to check for orphaned tables
child._validate_on_set_parent()

def _remove_child(self, child):
"""Remove a child Container. Intended for use in subclasses that allow dynamic addition of child Containers."""
Expand All @@ -442,6 +447,14 @@ def reset_parent(self):
else:
raise ValueError("Cannot reset parent when parent is not an AbstractContainer: %s" % repr(self.parent))

def _validate_on_set_parent(self):
"""Validate this Container after setting the parent.
This method is called by the parent setter. It can be overridden in subclasses to perform additional
validation. The default implementation does nothing.
"""
pass


class Container(AbstractContainer):
"""A container that can contain other containers and has special functionality for printing."""
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/common/test_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,7 @@ def setUp(self):
super().setUp()

def setUpContainer(self):
multi_container = SimpleMultiContainer(name='multi', containers=[self.table, self.target_table])
multi_container = SimpleMultiContainer(name='multi', containers=[self.target_table, self.table])
return multi_container

def _get(self, arg):
Expand Down
12 changes: 12 additions & 0 deletions tests/unit/test_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,18 @@ def test_reset_parent_no_parent(self):
obj.reset_parent()
self.assertIsNone(obj.parent)

def test_get_ancestors(self):
"""Test that get_ancestors returns the correct ancestors.
"""
grandparent_obj = Container('obj1')
parent_obj = Container('obj2')
child_obj = Container('obj3')
parent_obj.parent = grandparent_obj
child_obj.parent = parent_obj
self.assertTupleEqual(grandparent_obj.get_ancestors(), tuple())
self.assertTupleEqual(parent_obj.get_ancestors(), (grandparent_obj, ))
self.assertTupleEqual(child_obj.get_ancestors(), (parent_obj, grandparent_obj))


class TestHTMLRepr(TestCase):

Expand Down

0 comments on commit 0dd9dd0

Please sign in to comment.