Skip to content

Commit

Permalink
Add missing support for __columns__ having index=int (#605)
Browse files Browse the repository at this point in the history
* Add missing support for __columns__ having index=int

* Add tests

* Fix error when index=True

* Update changelog

* Fix test

* Fix tests
  • Loading branch information
rly authored May 12, 2021
1 parent 8493f96 commit 6be6692
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 18 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# HDMF Changelog

## HDMF 2.5.3 (May 12, 2021)

### Bug fix
- Fix issue where tables with multi-indexed columns defined using `__columns__` did not have attributes properly set.
@rly (#605)

## HDMF 2.5.2 (May 11, 2021)

### Bug fix
Expand Down
20 changes: 16 additions & 4 deletions src/hdmf/common/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -467,9 +467,17 @@ def _init_class_columns(self):

# set the table attributes for not yet init optional predefined columns
setattr(self, col['name'], None)
if col.get('index', False):
self.__uninit_cols[col['name'] + '_index'] = col
setattr(self, col['name'] + '_index', None)
index = col.get('index', False)
if index is not False:
if index is True:
index = 1
if isinstance(index, int):
assert index > 0, ValueError("integer index value must be greater than 0")
index_name = col['name']
for i in range(index):
index_name = index_name + '_index'
self.__uninit_cols[index_name] = col
setattr(self, index_name, None)
if col.get('enum', False):
self.__uninit_cols[col['name'] + '_elements'] = col
setattr(self, col['name'] + '_elements', None)
Expand All @@ -487,7 +495,11 @@ def __build_columns(columns, df=None):
data = None
if df is not None:
data = list(df[name].values)
if d.get('index', False):
index = d.get('index', False)
if index is not False:
if isinstance(index, int) and index > 1:
raise ValueError('Creating nested index columns using this method is not yet supported. Use '
'add_column or define the columns using __columns__ instead.')
index_data = None
if data is not None:
index_data = [len(data[0])]
Expand Down
87 changes: 73 additions & 14 deletions tests/unit/common/test_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,31 +222,89 @@ def test_add_column_multi_index(self):
]
)

def test_auto_multi_index_required(self):

class TestTable(DynamicTable):
__columns__ = (dict(name='qux', description='qux column', index=3, required=True),)

table = TestTable('table_name', 'table_description')
self.assertIsInstance(table.qux, VectorData) # check that the attribute is set
self.assertIsInstance(table.qux_index, VectorIndex) # check that the attribute is set
self.assertIsInstance(table.qux_index_index, VectorIndex) # check that the attribute is set
self.assertIsInstance(table.qux_index_index_index, VectorIndex) # check that the attribute is set
table.add_row(
qux=[
[
[1, 2, 3],
[1, 2, 3, 4]
]
]
)
table.add_row(
qux=[
[
[1, 2]
]
]
)

expected = [
[
[
[1, 2, 3],
[1, 2, 3, 4]
]
],
[
[
[1, 2]
]
]
]
self.assertListEqual(table['qux'][:], expected)
self.assertEqual(table.qux_index_index_index.data, [1, 2])

def test_auto_multi_index(self):

class TestTable(DynamicTable):
__columns__ = (dict(name='qux', description='qux column', index=2),)
__columns__ = (dict(name='qux', description='qux column', index=3),) # this is optional

table = TestTable('table_name', 'table_description')
table.add_row(qux=[
[1, 2, 3],
[1, 2, 3, 4]
])
table.add_row(qux=[
[1, 2]
]
)
self.assertIsNone(table.qux) # these are reserved as attributes but not yet initialized
self.assertIsNone(table.qux_index)
self.assertIsNone(table.qux_index_index)
self.assertIsNone(table.qux_index_index_index)
table.add_row(
qux=[
[
[1, 2, 3],
[1, 2, 3, 4]
]
]
)
table.add_row(
qux=[
[
[1, 2]
]
]
)

expected = [
[
[1, 2, 3],
[1, 2, 3, 4]
[
[1, 2, 3],
[1, 2, 3, 4]
]
],
[
[1, 2]
[
[1, 2]
]
]
]
self.assertListEqual(table['qux'][:], expected)
self.assertEqual(table.qux_index_index_index.data, [1, 2])

def test_getitem_row_num(self):
table = self.with_spec()
Expand Down Expand Up @@ -617,9 +675,10 @@ def setUpContainer(self):
table.add_column('bar', 'a float column')
table.add_column('baz', 'a string column')
table.add_column('qux', 'a boolean column')
table.add_column('corge', 'a doubly indexed int column', index=2)
table.add_column('quux', 'an enum column', enum=True)
table.add_row(foo=27, bar=28.0, baz="cat", qux=True, quux='a')
table.add_row(foo=37, bar=38.0, baz="dog", qux=False, quux='b')
table.add_row(foo=27, bar=28.0, baz="cat", corge=[[1, 2, 3], [4, 5, 6]], qux=True, quux='a')
table.add_row(foo=37, bar=38.0, baz="dog", corge=[[11, 12, 13], [14, 15, 16]], qux=False, quux='b')
return table

def test_index_out_of_bounds(self):
Expand Down

0 comments on commit 6be6692

Please sign in to comment.