diff --git a/CHANGELOG.md b/CHANGELOG.md index 8693a8029..000f22f45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ - ``DynamicTable.get_meanings_for_column`` (and the ``AlignedDynamicTable`` override) now returns ``None`` when the named column exists but has no ``MeaningsTable``, and raises ``KeyError`` only when the column itself does not exist. @rly [#1538](https://github.com/hdmf-dev/hdmf/pull/1538) ### Fixed +- Fixed `IndexError` when selecting an empty region from an in-memory `DynamicTableRegion` (e.g. `table["region"][i]` for a ragged region row that references no target rows, or an empty slice), both when the target table's columns hold their data as numpy arrays and when the target table has ragged columns. @h-mayorquin [#1549](https://github.com/hdmf-dev/hdmf/pull/1549) - Fixed the Jupyter HTML representation (`_repr_html_`) rendering a scalar numpy `bool` or `int` attribute (e.g. `np.bool_`, `np.int64`, as read back from an HDF5 attribute) as an expandable "array" block reporting `Shape: ()`, while `float` and `str` scalars rendered inline. `_unwrap_scalar` now also unwraps numpy scalars (`np.generic`) so every scalar renders inline consistently. @h-mayorquin [#1546](https://github.com/hdmf-dev/hdmf/pull/1546) - Fixed subclassing a `MultiContainerInterface` type without redefining `__init__`. A subclass now inherits the ancestor's constructor (custom or auto-generated) instead of regenerating one that drops the parent's docval arguments. This restores the common "generate with `get_class`, subclass to customize one method, re-register with `@register_class`" extension idiom for `DynamicTable`-family subtypes. @rly [#1540](https://github.com/hdmf-dev/hdmf/pull/1540) - Fixed `ObjectMapper._parse_isoformat` raising `ValueError: Invalid isoformat string` when reading an ISO 8601 timestamp that ends in the `Z` (UTC) designator on Python < 3.11. `datetime.fromisoformat` did not accept a trailing `Z` until Python 3.11, but `requires-python` is `>=3.10` and hdmf's own writer emits the equivalent `+00:00` offset, so a `Z`-terminated timestamp written by a peer tool was unreadable on the supported 3.10 floor. A trailing `Z`/`z` is now normalized to `+00:00` before parsing. @Leonard013 [#1539](https://github.com/hdmf-dev/hdmf/pull/1539) diff --git a/src/hdmf/common/table.py b/src/hdmf/common/table.py index 5c5004452..ce18194a4 100644 --- a/src/hdmf/common/table.py +++ b/src/hdmf/common/table.py @@ -241,7 +241,7 @@ def get(self, arg, **kwargs): if isinstance(arg, slice): indices = list(range(*arg.indices(_get_length(self.data)))) else: - if isinstance(arg[0], (bool, np.bool_)): + if len(arg) > 0 and isinstance(arg[0], (bool, np.bool_)): arg = np.where(arg)[0] indices = arg ret = list() @@ -1684,7 +1684,12 @@ def get(self, arg, index=False, df=True, **kwargs): # # When not returning a DataFrame, we need to recursively sort the subelements # of the list we are returning. This is carried out by the recursive method _index_lol - uniq = np.unique(ret) + # + # Region data are row indices, so the unique elements are cast to an integer dtype. + # This matters when `ret` is empty (a region row that references no target rows): + # np.unique of an empty list is float64, which is not a valid index into a column + # whose data is a numpy array. + uniq = np.unique(ret).astype(np.int64, copy=False) lut = {val: i for i, val in enumerate(uniq)} values = self.table.get(uniq, df=df, index=index, **kwargs) if df: diff --git a/tests/unit/common/test_table.py b/tests/unit/common/test_table.py index b52ca5617..c724e3a3e 100644 --- a/tests/unit/common/test_table.py +++ b/tests/unit/common/test_table.py @@ -1345,6 +1345,18 @@ def with_columns_and_data(self): ] return DynamicTable(name="with_columns_and_data", description='a test table', columns=columns) + def with_array_columns(self): + """Build a table whose columns hold their data as numpy arrays. + + ``Data.get`` passes the selection straight to a numpy-backed column, so the dtype of the + index array has to be a dtype numpy accepts as an index. + """ + columns = [ + VectorData(name='foo', description='foo column', data=np.array([1, 2, 3, 4, 5])), + VectorData(name='bar', description='bar column', data=np.array([10.0, 20.0, 30.0, 40.0, 50.0])), + ] + return DynamicTable(name='with_array_columns', description='a test table', columns=columns) + def test_indexed_dynamic_table_region(self): table = self.with_columns_and_data() dynamic_table_region = DynamicTableRegion(name='dtr', data=[1, 2, 2], description='desc', table=table) @@ -1413,6 +1425,40 @@ def test_dynamic_table_region_getitem_slice_of_column(self): res = dynamic_table_region[1:3, 'baz'] self.assertListEqual(res, ['dog', 'bird']) + def test_dynamic_table_region_getitem_empty_slice(self): + table = self.with_array_columns() + dynamic_table_region = DynamicTableRegion(name='dtr', data=[0, 1, 2], description='desc', table=table) + res = dynamic_table_region[1:1] + self.assertEqual(len(res), 0) + self.assertListEqual(list(res.columns), ['foo', 'bar']) + + def test_indexed_dynamic_table_region_getitem_empty_row(self): + target_table = self.with_array_columns() + table = DynamicTable(name='table', description='a test table') + table.add_column(name='dtr', description='indexed DynamicTableRegion', index=True, table=target_table) + table.add_row(dtr=[0]) + table.add_row(dtr=[]) + + self.assertEqual(len(table['dtr'][0]), 1) + res = table['dtr'][1] + self.assertEqual(len(res), 0) + self.assertListEqual(list(res.columns), ['foo', 'bar']) + + def test_indexed_dynamic_table_region_getitem_empty_row_ragged_target(self): + target_table = DynamicTable(name='target_table', description='a test table') + target_table.add_column(name='qux', description='a ragged column', index=True) + target_table.add_row(qux=[1, 2]) + target_table.add_row(qux=[3]) + table = DynamicTable(name='table', description='a test table') + table.add_column(name='dtr', description='indexed DynamicTableRegion', index=True, table=target_table) + table.add_row(dtr=[0]) + table.add_row(dtr=[]) + + self.assertEqual(len(table['dtr'][0]), 1) + res = table['dtr'][1] + self.assertEqual(len(res), 0) + self.assertListEqual(list(res.columns), ['qux']) + def test_dynamic_table_region_getitem_bad_index(self): table = self.with_columns_and_data() dynamic_table_region = DynamicTableRegion(name='dtr', data=[0, 1, 2, 2], description='desc', table=table) @@ -2751,6 +2797,15 @@ def test_get_with_boolean_array(self): self.assertEqual(result, [['a', 'b',], ['d', 'e']]) self.assertEqual(len(result), 2) + def test_get_with_empty_selection(self): + """Test VectorIndex.get with an empty list, np.array, and slice""" + data = VectorData(name='data', description='desc', data=['a', 'b', 'c', 'd', 'e']) + index = VectorIndex(name='index', data=[2, 3, 5], target=data) + + self.assertEqual(index.get([]), []) + self.assertEqual(index.get(np.array([], dtype=np.int64)), []) + self.assertEqual(index.get(slice(1, 1)), []) + def test_get_target_data_single_index(self): """Test get_target_data returns the VectorData for a single ragged array.""" foo = VectorData(name='foo', description='foo column', data=['a', 'b', 'c'])