diff --git a/rio/components/table.py b/rio/components/table.py index 5c63819c..f1cb7693 100644 --- a/rio/components/table.py +++ b/rio/components/table.py @@ -68,18 +68,43 @@ def _index_to_start_and_extent( This function is intentionally a separate function instead of a method to allow for easy unit-testing. """ + # Python considers booleans to be integers. Guard against them being used as + # numbers here + if isinstance(index, bool): + raise ValueError( + f"Table indices should be integers or slices, not {index!r}" + ) + # Integers are easy if isinstance(index, int): start = index - extent = 1 + stop = index + 1 # Slices need more work elif isinstance(index, slice): - start = 0 if index.start is None else index.start - stop = size_in_axis if index.stop is None else index.stop - extent = stop - start + if index.start is None: + start = 0 + elif isinstance(index.start, int) and not isinstance(index.start, bool): + start = index.start + else: + raise ValueError( + f"Table indices should be integers or slices, not {index!r}" + ) + + if index.stop is None: + stop = size_in_axis + elif isinstance(index.stop, int) and not isinstance(index.stop, bool): + stop = index.stop + else: + raise ValueError( + f"Table indices should be integers or slices, not {index!r}" + ) # Anything else is invalid + else: + raise ValueError( + f"Table indices should be integers or slices, not {index!r}" + ) # Negative numbers count backwards from the end if start < 0: @@ -90,8 +115,16 @@ def _index_to_start_and_extent( start = size_in_axis + start + if stop < 0: + if stop + size_in_axis < 0: + raise IndexError( + f"Negative index {stop} out of bounds for axis of size {size_in_axis}" + ) + + stop = size_in_axis + stop + # Done - return start, extent + return start, stop - start def _string_index_to_start_and_extent( @@ -102,12 +135,6 @@ def _string_index_to_start_and_extent( """ Same as `_index_to_start_and_extent`, but with support for string indices. """ - # Just a single integer is invalid - if isinstance(index, int): - raise ValueError( - f"Indices into tables should be two-dimensional. Did you mean `[{index}, :]`?" - ) - # If passed a string, select the entire column if isinstance(index, str): if column_names is None: @@ -137,6 +164,15 @@ def _indices_to_rectangle( index_y = slice(None) index_x = index else: + if not isinstance(index, tuple): + raise ValueError( + f"Table indices should be a tuple of two elements, not {index!r}" + ) + elif len(index) != 2: + raise ValueError( + f"Table indices should have exactly two elements, not {len(index)}" + ) + index_y, index_x = index # Get the index as a tuple (top, left, height, width) @@ -151,7 +187,7 @@ def _indices_to_rectangle( data_width, ) - return top, left, height, width + return left, top, width, height @final @@ -301,7 +337,12 @@ class Table(FundamentalComponent): right = left + width bottom = top + height - if top < 0 or left < 0 or bottom > data_height or right > data_width: + if ( + (top < 0 or top >= data_height) + or (left < 0 or left >= data_width) + or (bottom < 0 or bottom > data_height) + or (right < 0 or right > data_width) + ): raise IndexError( f"Table index out of bounds. You're trying to select [{top}:{bottom}, {left}:{right}] but the table is only {data_height}x{data_width}" ) diff --git a/rio/debug/dev_tools/layout_explainer.py b/rio/debug/dev_tools/layout_explainer.py index f022f303..102dfefb 100644 --- a/rio/debug/dev_tools/layout_explainer.py +++ b/rio/debug/dev_tools/layout_explainer.py @@ -322,6 +322,12 @@ class LayoutExplainer: f"The component has `grow_{axis_xy}=True` set, but it is placed inside of a `{type(self._parent).__name__}`. {type(self._parent).__name__} components can not make use of this property, so it has no effect." ) + # Warn if the component is aligned, but has no natural size + if alignment is not None and natural_size < 0.1: + self.warnings.append( + f"The component is aligned using `align_{axis_xy}` but it has no natural {axis_name}. Since aligned components receive the minimum amount of space necessary, and this component doesn't require any space, it will not be visible." + ) + # How much space did the parent hand down? result = io.StringIO() result.write( diff --git a/tests/test_table_indexing.py b/tests/test_table_indexing.py index cd761133..141293e3 100644 --- a/tests/test_table_indexing.py +++ b/tests/test_table_indexing.py @@ -26,7 +26,7 @@ make_index = MakeIndex() ( make_index[1:3, 4:7], False, - (1, 4, 2, 3), + (4, 1, 3, 2), ), # Selecting an entire column ( @@ -55,7 +55,7 @@ make_index = MakeIndex() ( make_index[1:3, "a"], True, - (1, 0, 2, 1), + (0, 1, 1, 2), ), # Attempting to select by name in the wrong axis ( @@ -88,47 +88,47 @@ make_index = MakeIndex() ( make_index[1.0], False, - TypeError, + ValueError, ), ( make_index[1.0, 2.0], False, - TypeError, + ValueError, ), ( make_index[1.0, 2.0, 3.0], False, - TypeError, + ValueError, ), ( make_index[True], False, - TypeError, + ValueError, ), ( make_index[True, False], False, - TypeError, + ValueError, ), ( make_index[True, False, True], False, - TypeError, + ValueError, ), ( make_index[None], False, - TypeError, + ValueError, ), ( make_index[None, None], False, - TypeError, + ValueError, ), ( make_index[None, None, None], False, - TypeError, + ValueError, ), # Range selection ( @@ -139,43 +139,43 @@ make_index = MakeIndex() ( make_index[5:, :], False, - (5, 0, 5, 20), + (0, 5, 10, 15), ), ( make_index[:5, :], False, - (0, 0, 5, 20), + (0, 0, 10, 5), ), ( make_index[:, 5:], False, - (0, 5, 10, 15), + (5, 0, 5, 20), ), ( make_index[:, :5], False, - (0, 0, 10, 5), + (0, 0, 5, 20), ), # Range selection, but negative ( make_index[-2:, :], False, - (8, 0, 2, 20), + (0, 18, 10, 2), ), ( make_index[:-2, :], False, - (0, 0, 8, 20), + (0, 0, 10, 18), ), ( make_index[:, -2:], False, - (0, 18, 10, 2), + (8, 0, 2, 20), ), ( make_index[:, :-2], False, - (0, 0, 10, 18), + (0, 0, 8, 20), ), # Out of bounds ( @@ -209,6 +209,27 @@ make_index = MakeIndex() False, IndexError, ), + # Incorrect types inside of slices + ( + make_index[2.0:, :], + False, + ValueError, + ), + ( + make_index[:True, :], + False, + ValueError, + ), + ( + make_index[:, "Heyho":], + False, + ValueError, + ), + ( + make_index[:, :list], + False, + ValueError, + ), ], ) def test_indices(