Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accumulation table bug fix #982

Merged
merged 16 commits into from
Dec 2, 2023
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 1 addition & 4 deletions docs/debug/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions python_ta/debug/accumulation_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
124 changes: 124 additions & 0 deletions tests/test_debug/test_accumulation_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Test suite for the AccumulationTable class on different
types of accumulator loops
"""
import copy

import pytest

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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])
Expand All @@ -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
Expand Down
Loading