Skip to content

Commit

Permalink
Fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@rly
rly committed Jul 25, 2023
1 parent 0dd9dd0 commit 36c826a
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 39 deletions.
15 changes: 12 additions & 3 deletions src/hdmf/common/table.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1422,12 +1422,21 @@ def __repr__(self):
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.
# when this DynamicTableRegion is added to a parent, check:
# 1) if the table was read from a written file, no need to validate further
p = self.table
while p is not None:
if p.container_source is not None:
return super()._validate_on_set_parent()

Check warning on line 1430 in src/hdmf/common/table.py

View check run for this annotation

Codecov / codecov/patch

src/hdmf/common/table.py#L1430 

Added line #L1430 was not covered by tests
p = p.parent

# 2) if none of the ancestors are ancestors of the linked-to table, then when this is written, the table
# field will point to a table that is not in the file
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 "
msg = (f"The linked table for DynamicTableRegion '{self.name}' does not share an ancestor with the "
"DynamicTableRegion.")
warn(msg)
return super()._validate_on_set_parent()
Expand Down
90 changes: 54 additions & 36 deletions tests/unit/common/test_linkedtables.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Module for testing functions specific to tables containing DynamicTableRegion columns
"""

import warnings
import numpy as np
from hdmf.common import DynamicTable, AlignedDynamicTable, VectorData, DynamicTableRegion, VectorIndex
from hdmf.testing import TestCase
Expand Down Expand Up @@ -139,11 +140,16 @@ def setUp(self):
description='filter value',
index=False)
# Aligned table
self.aligned_table = AlignedDynamicTable(name='my_aligned_table',
description='my test table',
columns=[VectorData(name='a1', description='a1', data=np.arange(3)), ],
colnames=['a1', ],
category_tables=[self.category0, self.category1])
with warnings.catch_warnings():
msg = "The linked table for DynamicTableRegion '.*' does not share an ancestor with the DynamicTableRegion."
warnings.filterwarnings("ignore", category=UserWarning, message=msg)
self.aligned_table = AlignedDynamicTable(
name='my_aligned_table',
description='my test table',
columns=[VectorData(name='a1', description='a1', data=np.arange(3)), ],
colnames=['a1', ],
category_tables=[self.category0, self.category1]
)

def tearDown(self):
del self.table_level0_0
Expand Down Expand Up @@ -241,13 +247,16 @@ def test_get_foreign_column_in_main_and_category_table(self):
columns=[VectorData(name='c1', description='c1', data=np.arange(4)),
DynamicTableRegion(name='c2', description='c2',
data=np.arange(4), table=temp_table0)])
temp_aligned_table = AlignedDynamicTable(name='my_aligned_table',
description='my test table',
category_tables=[temp_table],
colnames=['a1', 'a2'],
columns=[VectorData(name='a1', description='c1', data=np.arange(4)),
DynamicTableRegion(name='a2', description='c2',
data=np.arange(4), table=temp_table)])
with warnings.catch_warnings():
msg = "The linked table for DynamicTableRegion '.*' does not share an ancestor with the DynamicTableRegion."
warnings.filterwarnings("ignore", category=UserWarning, message=msg)
temp_aligned_table = AlignedDynamicTable(name='my_aligned_table',
description='my test table',
category_tables=[temp_table],
colnames=['a1', 'a2'],
columns=[VectorData(name='a1', description='c1', data=np.arange(4)),
DynamicTableRegion(name='a2', description='c2',
data=np.arange(4), table=temp_table)])
# We should get both the DynamicTableRegion from the main table and the category 't1'
self.assertListEqual(temp_aligned_table.get_foreign_columns(), [(None, 'a2'), ('t1', 'c2')])
# We should only get the column from the main table
Expand Down Expand Up @@ -275,12 +284,15 @@ def test_get_linked_tables_none(self):
colnames=['c1', 'c2'],
columns=[VectorData(name='c1', description='c1', data=np.arange(4)),
VectorData(name='c2', description='c2', data=np.arange(4))])
temp_aligned_table = AlignedDynamicTable(name='my_aligned_table',
description='my test table',
category_tables=[temp_table],
colnames=['a1', 'a2'],
columns=[VectorData(name='a1', description='c1', data=np.arange(4)),
VectorData(name='a2', description='c2', data=np.arange(4))])
with warnings.catch_warnings():
msg = "The linked table for DynamicTableRegion '.*' does not share an ancestor with the DynamicTableRegion."
warnings.filterwarnings("ignore", category=UserWarning, message=msg)
temp_aligned_table = AlignedDynamicTable(name='my_aligned_table',
description='my test table',
category_tables=[temp_table],
colnames=['a1', 'a2'],
columns=[VectorData(name='a1', description='c1', data=np.arange(4)),
VectorData(name='a2', description='c2', data=np.arange(4))])
self.assertListEqual(temp_aligned_table.get_linked_tables(), [])
self.assertListEqual(temp_aligned_table.get_linked_tables(ignore_category_tables=True), [])

Expand All @@ -294,13 +306,16 @@ def test_get_linked_tables_complex_link(self):
columns=[VectorData(name='c1', description='c1', data=np.arange(4)),
DynamicTableRegion(name='c2', description='c2',
data=np.arange(4), table=temp_table0)])
temp_aligned_table = AlignedDynamicTable(name='my_aligned_table',
description='my test table',
category_tables=[temp_table],
colnames=['a1', 'a2'],
columns=[VectorData(name='a1', description='c1', data=np.arange(4)),
DynamicTableRegion(name='a2', description='c2',
data=np.arange(4), table=temp_table)])
with warnings.catch_warnings():
msg = "The linked table for DynamicTableRegion '.*' does not share an ancestor with the DynamicTableRegion."
warnings.filterwarnings("ignore", category=UserWarning, message=msg)
temp_aligned_table = AlignedDynamicTable(name='my_aligned_table',
description='my test table',
category_tables=[temp_table],
colnames=['a1', 'a2'],
columns=[VectorData(name='a1', description='c1', data=np.arange(4)),
DynamicTableRegion(name='a2', description='c2',
data=np.arange(4), table=temp_table)])
# NOTE: in this example templ_aligned_table both points to temp_table and at the
# same time contains temp_table as a category. This could lead to temp_table
# visited multiple times and we want to make sure this doesn't happen
Expand All @@ -326,17 +341,20 @@ def test_get_linked_tables_simple_link(self):
columns=[VectorData(name='c1', description='c1', data=np.arange(4)),
VectorData(name='c2', description='c2', data=np.arange(4))])
temp_table = DynamicTable(name='t1', description='t1',
colnames=['c1', 'c2'],
columns=[VectorData(name='c1', description='c1', data=np.arange(4)),
DynamicTableRegion(name='c2', description='c2',
data=np.arange(4), table=temp_table0)])
temp_aligned_table = AlignedDynamicTable(name='my_aligned_table',
description='my test table',
category_tables=[temp_table],
colnames=['a1', 'a2'],
columns=[VectorData(name='a1', description='c1', data=np.arange(4)),
DynamicTableRegion(name='a2', description='c2',
data=np.arange(4), table=temp_table0)])
colnames=['c1', 'c2'],
columns=[VectorData(name='c1', description='c1', data=np.arange(4)),
DynamicTableRegion(name='c2', description='c2',
data=np.arange(4), table=temp_table0)])
with warnings.catch_warnings():
msg = "The linked table for DynamicTableRegion '.*' does not share an ancestor with the DynamicTableRegion."
warnings.filterwarnings("ignore", category=UserWarning, message=msg)
temp_aligned_table = AlignedDynamicTable(name='my_aligned_table',
description='my test table',
category_tables=[temp_table],
colnames=['a1', 'a2'],
columns=[VectorData(name='a1', description='c1', data=np.arange(4)),
DynamicTableRegion(name='a2', description='c2',
data=np.arange(4), table=temp_table0)])
# NOTE: in this example temp_aligned_table and temp_table both point to temp_table0
# We should get both the DynamicTableRegion from the main table and the category 't1'
linked_tables = temp_aligned_table.get_linked_tables()
Expand Down

0 comments on commit 36c826a

Please sign in to comment.