From ea21cad4032569e8c51e2a27c44bac0d6649a53c Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Mon, 6 Oct 2025 14:03:15 -0700 Subject: [PATCH 01/13] remove extra html repr output --- src/hdmf/common/table.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/hdmf/common/table.py b/src/hdmf/common/table.py index f4b2cd011..6be229c79 100644 --- a/src/hdmf/common/table.py +++ b/src/hdmf/common/table.py @@ -1288,10 +1288,8 @@ def generate_html_repr(self, level: int = 0, access_code: str = "", nrows: int = inside = f"{self[:min(nrows, len(self))].to_html()}" - if len(self) == nrows + 1: - inside += "
... and 1 more row.
" - elif len(self) > nrows + 1: - inside += f"... and {len(self) - nrows} more rows.
" + if len(self) >= nrows + 1: + inside += f"... and {len(self) - nrows} more row(s).
" out += ( f'\n ' ' | foo | \nbar | \nbaz | \n
---|---|---|---|
id | \n ' - '\n | \n | \n |
... and 1 more row(s).
' ) @@ -1334,6 +1349,28 @@ def test_no_df_nested(self): with self.assertRaisesWith(ValueError, msg): dynamic_table_region.get(0, df=False, index=False) + def test_create_region_with_valid_slice_range(self): + table = self.with_columns_and_data() + region = table.create_region(name='region', region=slice(0, 2), description='test region') + self.assertEqual(region.data, [0, 1]) + + def test_create_region_with_none_slice(self): + table = self.with_columns_and_data() + region = table.create_region(name='region2', region=slice(0, None), description='test region') + self.assertEqual(region.data, [0, 1, 2, 3, 4]) + + def test_create_region_with_negative_index(self): + table = self.with_columns_and_data() + + msg = 'The index -1 is out of range for this DynamicTable of length 5' + with self.assertRaisesWith(IndexError, msg): + table.create_region(name='region', region=[-1, 0], description='test region') + + def test_create_region_with_out_of_range_index(self): + table = self.with_columns_and_data() + msg = 'The index 10 is out of range for this DynamicTable of length 5' + with self.assertRaisesWith(IndexError, msg): + table.create_region(name='region', region=[0, 10], description='test region') class DynamicTableRegionRoundTrip(H5RoundTripMixin, TestCase): @@ -2512,6 +2549,23 @@ def test_init_data(self): self.assertListEqual(foo_ind[0], ['a', 'b']) self.assertListEqual(foo_ind[1], ['c']) + def test_get_with_boolean(self): + """Test VectorIndex.get with boolean argument""" + data = VectorData(name='data', description='desc', data=['a', 'b', 'c', 'd', 'e']) + index = VectorIndex(name='index', data=[2, 3, 5], target=data) + result = index.get([True, False, True]) + + self.assertEqual(result, [['a', 'b',], ['d', 'e']]) + self.assertEqual(len(result), 2) + + def test_get_with_boolean_array(self): + """Test VectorIndex.get with boolean np.array argument""" + data = VectorData(name='data', description='desc', data=['a', 'b', 'c', 'd', 'e']) + index = VectorIndex(name='index', data=[2, 3, 5], target=data) + result = index.get(np.array([True, False, True])) + + self.assertEqual(result, [['a', 'b',], ['d', 'e']]) + self.assertEqual(len(result), 2) class TestDoubleIndex(TestCase): @@ -2659,6 +2713,14 @@ def test_enum_index(self): index=pd.Series(name='id', data=[0, 1, 2])) pd.testing.assert_frame_equal(exp, rec) + def test_add_column_table_and_enum_error(self): + """Test that adding a column with both table and enum raises an error.""" + table = DynamicTable(name='table0', description='an example table') + + msg = "column 'col1' cannot be both a table region and come from an enumerable set of elements" + with self.assertRaisesWith(ValueError, msg): + table.add_column(name='col1', description='test', table=True, enum=True) + class TestDynamicTableInitIndexRoundTrip(H5RoundTripMixin, TestCase): From 23f570c79cbd9e7a4a6d74a7ad3407325b832773 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 6 Oct 2025 23:47:37 +0000 Subject: [PATCH 06/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/unit/common/test_common.py | 2 +- tests/unit/common/test_table.py | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/unit/common/test_common.py b/tests/unit/common/test_common.py index 4d1334f01..39a8a69b5 100644 --- a/tests/unit/common/test_common.py +++ b/tests/unit/common/test_common.py @@ -39,7 +39,7 @@ def test_available_namespaces(self): def test_extensions_type_map(self): # TODO - update to error when updating to HDMF 5.0 existing_type_map = get_type_map() - with self.assertWarnsWith(DeprecationWarning, + with self.assertWarnsWith(DeprecationWarning, "The 'extensions' argument is deprecated and will be removed in HDMF 5.0"): get_type_map(extensions=existing_type_map) diff --git a/tests/unit/common/test_table.py b/tests/unit/common/test_table.py index 4ace575bf..a6e145469 100644 --- a/tests/unit/common/test_table.py +++ b/tests/unit/common/test_table.py @@ -896,19 +896,19 @@ def test_from_dataframe_missing_required_columns(self): def test_build_columns_with_nested_index_error(self): """Test that building columns with nested index > 1 raises an error""" df = pd.DataFrame({'col1': [1, 2, 3, 4, 5],}) - + msg = ('Creating nested index columns using this method is not yet supported. ' 'Use add_column or define the columns using __columns__ instead.') with self.assertRaisesWith(ValueError, msg): - DynamicTable.from_dataframe(df, 'test', + DynamicTable.from_dataframe(df, 'test', columns=([{'name': 'col1', 'description': 'optional column', 'index': 2},])) def test_build_columns_with_enum(self): """Test that building columns with enum as true creates an Enum column""" # TODO - diffiult to trigger empty enum data, add test if possible df = pd.DataFrame({'col1': [1, 2, 3, 4, 5],}) - table = DynamicTable.from_dataframe(df, 'test', columns=([{'name': 'col1', - 'description': 'optional enum column', + table = DynamicTable.from_dataframe(df, 'test', columns=([{'name': 'col1', + 'description': 'optional enum column', 'enum': True},])) self.assertIsInstance(table['col1'], EnumData) @@ -1353,7 +1353,7 @@ def test_create_region_with_valid_slice_range(self): table = self.with_columns_and_data() region = table.create_region(name='region', region=slice(0, 2), description='test region') self.assertEqual(region.data, [0, 1]) - + def test_create_region_with_none_slice(self): table = self.with_columns_and_data() region = table.create_region(name='region2', region=slice(0, None), description='test region') @@ -1361,7 +1361,7 @@ def test_create_region_with_none_slice(self): def test_create_region_with_negative_index(self): table = self.with_columns_and_data() - + msg = 'The index -1 is out of range for this DynamicTable of length 5' with self.assertRaisesWith(IndexError, msg): table.create_region(name='region', region=[-1, 0], description='test region') @@ -2556,13 +2556,13 @@ def test_get_with_boolean(self): result = index.get([True, False, True]) self.assertEqual(result, [['a', 'b',], ['d', 'e']]) - self.assertEqual(len(result), 2) + self.assertEqual(len(result), 2) def test_get_with_boolean_array(self): """Test VectorIndex.get with boolean np.array argument""" data = VectorData(name='data', description='desc', data=['a', 'b', 'c', 'd', 'e']) index = VectorIndex(name='index', data=[2, 3, 5], target=data) - result = index.get(np.array([True, False, True])) + result = index.get(np.array([True, False, True])) self.assertEqual(result, [['a', 'b',], ['d', 'e']]) self.assertEqual(len(result), 2) @@ -2716,11 +2716,11 @@ def test_enum_index(self): def test_add_column_table_and_enum_error(self): """Test that adding a column with both table and enum raises an error.""" table = DynamicTable(name='table0', description='an example table') - + msg = "column 'col1' cannot be both a table region and come from an enumerable set of elements" with self.assertRaisesWith(ValueError, msg): table.add_column(name='col1', description='test', table=True, enum=True) - + class TestDynamicTableInitIndexRoundTrip(H5RoundTripMixin, TestCase): From 6b28a37a83a2ac39d4595d4c81ce0cceac95ef5c Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Tue, 7 Oct 2025 10:59:07 -0700 Subject: [PATCH 07/13] remove h5py<2 specific error --- src/hdmf/common/table.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/hdmf/common/table.py b/src/hdmf/common/table.py index 792bbd472..2b8266eac 100644 --- a/src/hdmf/common/table.py +++ b/src/hdmf/common/table.py @@ -1109,17 +1109,9 @@ def __get_selection_as_dict(self, arg, df, index, exclude=None, **kwargs): return ret # if index is out of range, different errors can be generated depending on the dtype of the column # but despite the differences, raise an IndexError from that error - except ValueError as ve: + except IndexError as ie: # in h5py <2, if the column is an h5py.Dataset, a ValueError was raised # in h5py 3+, this became an IndexError - x = re.match(r"^Index \((.*)\) out of range \(.*\)$", str(ve)) - if x: - msg = ("Row index %s out of range for %s '%s' (length %d)." - % (x.groups()[0], self.__class__.__name__, self.name, len(self))) - raise IndexError(msg) from ve - else: # pragma: no cover - raise ve - except IndexError as ie: x = re.match(r"^Index \((.*)\) out of range for \(.*\)$", str(ie)) if x: msg = ("Row index %s out of range for %s '%s' (length %d)." From d59fd8c4f9aa2a23cb184b575df5f3f77b926f91 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Tue, 7 Oct 2025 10:59:13 -0700 Subject: [PATCH 08/13] add additional tests --- tests/unit/common/test_table.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/unit/common/test_table.py b/tests/unit/common/test_table.py index a6e145469..80cca5de0 100644 --- a/tests/unit/common/test_table.py +++ b/tests/unit/common/test_table.py @@ -903,6 +903,23 @@ def test_build_columns_with_nested_index_error(self): DynamicTable.from_dataframe(df, 'test', columns=([{'name': 'col1', 'description': 'optional column', 'index': 2},])) + def test_build_columns_with_index(self): + """Test that building columns with index=True creates a VectorIndex column""" + ragged_list = [[1, 2], [3], [4, 5]] + df = pd.DataFrame({'col1': ragged_list,}) + + table = DynamicTable.from_dataframe(df, 'test', columns=([{'name': 'col1', 'description': 'optional column', 'index': True},])) + self.assertIsInstance(table['col1'], VectorData) + self.assertIsInstance(table['col1_index'], VectorIndex) + self.assertEqual(table['col1_index'][:], ragged_list) + + def test_build_columns_with_dynamic_table_region(self): + """Test that building columns with index=True creates a VectorIndex column""" + df = pd.DataFrame({'col1': list()},) + + table = DynamicTable.from_dataframe(df, 'test', columns=([{'name': 'col1', 'description': 'required region', 'required': True, 'table': True}])) + self.assertIsInstance(table['col1'], DynamicTableRegion) + def test_build_columns_with_enum(self): """Test that building columns with enum as true creates an Enum column""" # TODO - diffiult to trigger empty enum data, add test if possible @@ -1354,6 +1371,12 @@ def test_create_region_with_valid_slice_range(self): region = table.create_region(name='region', region=slice(0, 2), description='test region') self.assertEqual(region.data, [0, 1]) + def test_create_region_with_invalid_slice_range(self): + table = self.with_columns_and_data() + msg = 'region slice slice(-1, 2, None) is out of range for this DynamicTable of length 5' + with self.assertRaisesWith(IndexError, msg): + table.create_region(name='region2', region=slice(-1, 2), description='test region') + def test_create_region_with_none_slice(self): table = self.with_columns_and_data() region = table.create_region(name='region2', region=slice(0, None), description='test region') From 2155e250b096cc8abb210abb166664f9744a7cd9 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 7 Oct 2025 18:00:14 +0000 Subject: [PATCH 09/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/unit/common/test_table.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/common/test_table.py b/tests/unit/common/test_table.py index 80cca5de0..daf98805e 100644 --- a/tests/unit/common/test_table.py +++ b/tests/unit/common/test_table.py @@ -919,7 +919,7 @@ def test_build_columns_with_dynamic_table_region(self): table = DynamicTable.from_dataframe(df, 'test', columns=([{'name': 'col1', 'description': 'required region', 'required': True, 'table': True}])) self.assertIsInstance(table['col1'], DynamicTableRegion) - + def test_build_columns_with_enum(self): """Test that building columns with enum as true creates an Enum column""" # TODO - diffiult to trigger empty enum data, add test if possible From df7ba0bed809409378d3e23320b207741e8b32cd Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Tue, 7 Oct 2025 13:44:09 -0700 Subject: [PATCH 10/13] fix formatting --- tests/unit/common/test_table.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/unit/common/test_table.py b/tests/unit/common/test_table.py index daf98805e..7da7745ae 100644 --- a/tests/unit/common/test_table.py +++ b/tests/unit/common/test_table.py @@ -901,14 +901,18 @@ def test_build_columns_with_nested_index_error(self): 'Use add_column or define the columns using __columns__ instead.') with self.assertRaisesWith(ValueError, msg): DynamicTable.from_dataframe(df, 'test', - columns=([{'name': 'col1', 'description': 'optional column', 'index': 2},])) + columns=([{'name': 'col1', + 'description': 'optional column', + 'index': 2},])) def test_build_columns_with_index(self): """Test that building columns with index=True creates a VectorIndex column""" ragged_list = [[1, 2], [3], [4, 5]] df = pd.DataFrame({'col1': ragged_list,}) - table = DynamicTable.from_dataframe(df, 'test', columns=([{'name': 'col1', 'description': 'optional column', 'index': True},])) + table = DynamicTable.from_dataframe(df, 'test', columns=([{'name': 'col1', + 'description': 'optional column', + 'index': True},])) self.assertIsInstance(table['col1'], VectorData) self.assertIsInstance(table['col1_index'], VectorIndex) self.assertEqual(table['col1_index'][:], ragged_list) @@ -917,7 +921,11 @@ def test_build_columns_with_dynamic_table_region(self): """Test that building columns with index=True creates a VectorIndex column""" df = pd.DataFrame({'col1': list()},) - table = DynamicTable.from_dataframe(df, 'test', columns=([{'name': 'col1', 'description': 'required region', 'required': True, 'table': True}])) + table = DynamicTable.from_dataframe(df, 'test', + columns=([{'name': 'col1', + 'description': 'required region', + 'required': True, + 'table': True}])) self.assertIsInstance(table['col1'], DynamicTableRegion) def test_build_columns_with_enum(self): From 6400cc2250f3370f9b2e02859d3f1ba8b4ac7289 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Tue, 7 Oct 2025 13:45:21 -0700 Subject: [PATCH 11/13] fix formatting --- src/hdmf/common/table.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/hdmf/common/table.py b/src/hdmf/common/table.py index 2b8266eac..9d0072156 100644 --- a/src/hdmf/common/table.py +++ b/src/hdmf/common/table.py @@ -741,7 +741,6 @@ def add_row(self, **kwargs): for colname, colnum in self.__colids.items(): if colname not in data: - # NOTE: I think this cannot be triggered since it is checked in _validate_new_row, could be removed raise ValueError("column '%s' missing" % colname) col = self.__df_cols[colnum] if isinstance(col, VectorIndex): From b3530134eae08c589fe0f8c64a3b93c17be6e347 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 7 Oct 2025 20:45:45 +0000 Subject: [PATCH 12/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/unit/common/test_table.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/unit/common/test_table.py b/tests/unit/common/test_table.py index 7da7745ae..ebb86f1dc 100644 --- a/tests/unit/common/test_table.py +++ b/tests/unit/common/test_table.py @@ -901,8 +901,8 @@ def test_build_columns_with_nested_index_error(self): 'Use add_column or define the columns using __columns__ instead.') with self.assertRaisesWith(ValueError, msg): DynamicTable.from_dataframe(df, 'test', - columns=([{'name': 'col1', - 'description': 'optional column', + columns=([{'name': 'col1', + 'description': 'optional column', 'index': 2},])) def test_build_columns_with_index(self): @@ -910,8 +910,8 @@ def test_build_columns_with_index(self): ragged_list = [[1, 2], [3], [4, 5]] df = pd.DataFrame({'col1': ragged_list,}) - table = DynamicTable.from_dataframe(df, 'test', columns=([{'name': 'col1', - 'description': 'optional column', + table = DynamicTable.from_dataframe(df, 'test', columns=([{'name': 'col1', + 'description': 'optional column', 'index': True},])) self.assertIsInstance(table['col1'], VectorData) self.assertIsInstance(table['col1_index'], VectorIndex) @@ -921,10 +921,10 @@ def test_build_columns_with_dynamic_table_region(self): """Test that building columns with index=True creates a VectorIndex column""" df = pd.DataFrame({'col1': list()},) - table = DynamicTable.from_dataframe(df, 'test', - columns=([{'name': 'col1', - 'description': 'required region', - 'required': True, + table = DynamicTable.from_dataframe(df, 'test', + columns=([{'name': 'col1', + 'description': 'required region', + 'required': True, 'table': True}])) self.assertIsInstance(table['col1'], DynamicTableRegion) From 8729205cd2ee099053c3ab9cba85005f26e19db1 Mon Sep 17 00:00:00 2001 From: Steph Prince <40640337+stephprince@users.noreply.github.com> Date: Tue, 7 Oct 2025 14:10:40 -0700 Subject: [PATCH 13/13] remove test for deprecated extension kwarg --- tests/unit/common/test_common.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/unit/common/test_common.py b/tests/unit/common/test_common.py index 39a8a69b5..53fb5ebce 100644 --- a/tests/unit/common/test_common.py +++ b/tests/unit/common/test_common.py @@ -36,13 +36,6 @@ def test_available_namespaces(self): self.assertIn('hdmf-common', ns) self.assertIn('hdmf-experimental', ns) - def test_extensions_type_map(self): - # TODO - update to error when updating to HDMF 5.0 - existing_type_map = get_type_map() - with self.assertWarnsWith(DeprecationWarning, - "The 'extensions' argument is deprecated and will be removed in HDMF 5.0"): - get_type_map(extensions=existing_type_map) - def test_get_resources_legacy(self): """Test legacy _get_resources function.""" result = _get_resources()