diff --git a/CHANGELOG.md b/CHANGELOG.md index c0f27642b..c8c669e86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Both PlainReporter and ColorReporter emphasize specific code chunks by using overline characters under any part that is highlighted as ERROR. - Added new configuration option `allow-pylint-comments` to let users choose whether PythonTA should allow comments beginning with pylint: or not. - `AccumulationTable` can now track variables initialized within the `for` loop. Prior, only variables initialized before the `for` loop could be tracked. -- `AccumulationTable` can now take in any accumulator expressions, for eg. `x * 2`, instead of just variables +- `AccumulationTable` now stores deep copies of objects rather than shallow copies, thus fixing issues that come up in case of mutation during loop. +- `AccumulationTable` can now take in any accumulator expressions, for eg. `x * 2`, instead of just variables. ## [2.6.4] - 2024-11-10 diff --git a/docs/debug/index.md b/docs/debug/index.md index 28ef2b51a..932721c94 100644 --- a/docs/debug/index.md +++ b/docs/debug/index.md @@ -134,10 +134,7 @@ The `AccumulationTable` is a new PythonTA feature and currently has the followin 1. `AccumulationTable` uses [`sys.settrace`] to update variable state, and so is not compatible with other libraries (e.g. debuggers, code coverage tools). -2. Loop variable state is stored by creating shallow copies of the objects. - Loops that mutate a nested part of an object will not have their state displayed properly. - -3. The `AccumulationTable` context manager can only log the execution of one for loop. +2. The `AccumulationTable` context manager can only log the execution of one for loop. To log the state of multiple for loops, each must be wrapped in a separate `with` statement and fresh `AccumulationTable` instance. [tabulate]: https://github.com/astanin/python-tabulate diff --git a/python_ta/debug/accumulation_table.py b/python_ta/debug/accumulation_table.py index 56e19970b..985fb3ab3 100644 --- a/python_ta/debug/accumulation_table.py +++ b/python_ta/debug/accumulation_table.py @@ -88,19 +88,20 @@ def _record_iteration(self, frame: types.FrameType) -> None: """Record the values of the accumulator variables and loop variables of an iteration""" if self.loop_variables != {} and len(list(self.loop_variables.values())[0]) > 0: for loop_var in self.loop_variables: - self.loop_variables[loop_var].append(copy.copy(frame.f_locals[loop_var])) + self.loop_variables[loop_var].append(copy.deepcopy(frame.f_locals[loop_var])) else: for loop_var in self.loop_variables: self.loop_variables[loop_var].append(NO_VALUE) for accumulator in self.loop_accumulators: if accumulator in frame.f_locals: - value = copy.copy(frame.f_locals[accumulator]) + value = copy.deepcopy(frame.f_locals[accumulator]) elif accumulator in frame.f_code.co_varnames or accumulator in frame.f_code.co_names: value = NO_VALUE else: # name error wil be raised if accumulator cannot be found value = eval(accumulator, frame.f_globals, frame.f_locals) + value = copy.deepcopy(value) self.loop_accumulators[accumulator].append(value) diff --git a/tests/test_debug/test_accumulation_table.py b/tests/test_debug/test_accumulation_table.py index bb1447821..4d4d1e878 100644 --- a/tests/test_debug/test_accumulation_table.py +++ b/tests/test_debug/test_accumulation_table.py @@ -2,6 +2,7 @@ Test suite for the AccumulationTable class on different types of accumulator loops """ +import copy import pytest @@ -148,6 +149,56 @@ def test_five_nested_while_loop() -> None: } +def test_accumulation_table_list_deepcopy(): + data = [[1], [2], [3]] + with AccumulationTable(["data"]) as table: + for sublist in data: + sublist[0] *= 2 + recorded_value_0 = table.loop_accumulators["data"][0] + expected_value_0 = [[1], [2], [3]] + recorded_value_1 = table.loop_accumulators["data"][1] + expected_value_1 = [[2], [2], [3]] + recorded_value_2 = table.loop_accumulators["data"][2] + expected_value_2 = [[2], [4], [3]] + recorded_value_3 = table.loop_accumulators["data"][3] + expected_value_3 = [[2], [4], [6]] + assert recorded_value_0 == expected_value_0 + assert recorded_value_1 == expected_value_1 + assert recorded_value_2 == expected_value_2 + assert recorded_value_3 == expected_value_3 + + +def test_loop_variables_with_deepcopy(): + data = [[[1, 2], [3, 4]], [[5, 6], [7, 8]], [[9, 10], [11, 12]]] + + with AccumulationTable(["data"]) as table: + for nested_list in data: + nested_list[0][0] += 100 + + recorded_values = table.loop_variables["nested_list"] + expected_values = ["N/A", [[101, 2], [3, 4]], [[105, 6], [7, 8]], [[109, 10], [11, 12]]] + + assert recorded_values == expected_values + + +def test_accumulation_table_dict_deepcopy(): + data = {"variable": [{"nested": 1}, {"nested": 2}]} + + with AccumulationTable(["data"]) as table: + for item in data["variable"]: + item["nested"] *= 2 + + recorded_value_0 = table.loop_accumulators["data"][0] + expected_value_0 = {"variable": [{"nested": 1}, {"nested": 2}]} + recorded_value_1 = table.loop_accumulators["data"][1] + expected_value_1 = {"variable": [{"nested": 2}, {"nested": 2}]} + recorded_value_2 = table.loop_accumulators["data"][2] + expected_value_2 = {"variable": [{"nested": 2}, {"nested": 4}]} + assert recorded_value_0 == expected_value_0 + assert recorded_value_1 == expected_value_1 + assert recorded_value_2 == expected_value_2 + + class MyClass: items: list sum_so_far: int @@ -182,6 +233,69 @@ def accumulate_class_var(self) -> None: assert table.loop_variables == {"item": ["N/A", 10, 20, 30]} assert table.loop_accumulators == {"MyClass.difference_so_far": [0, -10, -30, -60]} + def check_accumulation_table_accumulator_deepcopy(self): + if any( + isinstance(sub, list) + for sublist in self.items + if isinstance(sublist, list) + for sub in sublist + ): + return ( + "Checking only for lists with max depth 2, because if that works, other depths will work too." + "Please provide list with max depth 2." + ) + + original_items = copy.deepcopy(self.items) + with AccumulationTable(["self.items"]) as table: + for sublist in self.items: + if isinstance(sublist, list): + sublist[0] *= 2 + for i in range(0, len(table.loop_accumulators["self.items"])): + recorded_value = table.loop_accumulators["self.items"][i] + expected_value = [] + if i != 0: + if isinstance(self.items[i - 1], list): + expected_value.extend(original_items[0 : i - 1]) + expected_value.append( + [original_items[i - 1][0] * 2] + original_items[i - 1][1:] + ) + expected_value.extend(original_items[i:]) + original_items = expected_value + else: + expected_value.extend(original_items) + else: + expected_value.extend(original_items) + assert recorded_value == expected_value + + def check_accumulation_table_loop_variable_deepcopy(self): + if any( + isinstance(sub, list) + for sublist in self.items + if isinstance(sublist, list) + for sub in sublist + ): + return ( + "Checking only for lists with max depth 2, because if that works, other depths will work too." + "Please provide list with max depth 2." + ) + + original_items = copy.deepcopy(self.items) + with AccumulationTable(["self.items"]) as table: + for nested_list in self.items: + if isinstance(nested_list, list): + nested_list[0] += 10 + recorded_values = table.loop_variables["nested_list"] + expected_values = [] + for i in range(0, len(original_items) + 1): + if i == 0: + expected_values.append("N/A") + continue + if not isinstance(original_items[i - 1], list): + expected_values.append(original_items[i - 1]) + else: + expected_values.append([original_items[i - 1][0] + 10] + original_items[i - 1][1:]) + assert recorded_values == expected_values + def test_class_var() -> None: my_class = MyClass([10, 20, 30]) @@ -198,6 +312,16 @@ def test_class_var_accumulator() -> None: my_class.accumulate_class_var() +def test_deepcopy_accumulator_in_class() -> None: + checker = MyClass([1, 2, [3, 4], [5], 7, 8]) + checker.check_accumulation_table_accumulator_deepcopy() + + +def test_deepcopy_loop_variables_in_class() -> None: + checker = MyClass([1, 2, [3, 4], [5], 7, 8]) + checker.check_accumulation_table_loop_variable_deepcopy() + + def test_expression_accumulator() -> None: test_list = [10, 20, 30] sum_so_far = 0