Recursion table #984
Recursion table #984
Conversation
Pull Request Test Coverage Report for Build 7159830368
💛 - Coveralls |
There was a problem hiding this comment.
@vsahni3 great work. I've left some comments for you to work on. In addition to those comments, please draft a new section of the documentation for RecursionTable, similar to AccumulationTable.
python_ta/debug/recursion_table.py
Outdated
|
|
||
|
|
||
| def clean_frame_variables(frame: types.FrameType) -> dict[str, Any]: | ||
| """remove the local variables from the frame's locals and keep only the |
There was a problem hiding this comment.
Docstring descriptions should be full sentences. This means you should capitalize the first letter and end with a period. Make sure to do this for all docstrings in this file.
python_ta/debug/recursion_table.py
Outdated
| """ | ||
| raw_variables = frame.f_locals | ||
| parameters = inspect.getargvalues(frame).args | ||
| # not mutating the local variables to avoid unintended effects |
There was a problem hiding this comment.
It is not clear what this comment is referring to
python_ta/debug/recursion_table.py
Outdated
| # we don't need to worry about the order of inputs changing | ||
| caller_val = f"{self.function_name}(" | ||
| count = 0 | ||
| for var in frame_variables: |
There was a problem hiding this comment.
Don't use a loop here, use the str.join method and a comprehension.
python_ta/debug/recursion_table.py
Outdated
| # this handles the very first function call | ||
| if not self.function_name: | ||
| self.function_name = frame.f_code.co_name | ||
| caller_func_string = "NA" |
There was a problem hiding this comment.
The string should be "N/A". Make this a constant at the top of this file.
python_ta/debug/recursion_table.py
Outdated
| # ignore the execution of the __exit__ method | ||
| if event == "call" and frame.f_code.co_name != "__exit__": | ||
| self._record_call(frame) | ||
| if event == "return": |
There was a problem hiding this comment.
prefer elif to consecutive ifs when conditions are mutually exclusive
python_ta/util/tree.py
Outdated
| def add_child(self, child_node): | ||
| self.children.append(child_node) | ||
|
|
||
| def check_tree_equality(self, tree1): |
There was a problem hiding this comment.
Name this method __eq__ and then in the tests below use == to check whether the trees are equal.
python_ta/debug/recursion_table.py
Outdated
| function_name: str | ||
| _trees: dict[types.FrameType, Tree] | ||
|
|
||
| def __init__(self) -> None: |
There was a problem hiding this comment.
Have the user enter the name of a function that they want to trace (this is more explicit than just looking at the first function name).
For example, with RecursionTable('fact'):
Also, for method calls the user should enter a qualified name like BinarySearchTree.__contains__, not just __contains__. You should be able to support this behaviour in your code below by using the co_qualname attribute instead of the co_name attribute for function code objects.
python_ta/debug/recursion_table.py
Outdated
| representing the function call | ||
| """ | ||
|
|
||
| frames_mapping: dict[types.FrameType, list] |
There was a problem hiding this comment.
First, change the name of the attribute to frame_data.
More involved change: Instead of the dict values being lists, have them be dictionaries that store the individual pieces of data in the same structure as the eventual returned table, plus an extra field for the computed string representation of the call.
More concretely, the type annotation should be dict[types.FrameType, dict[str, Any]]. For the fact example each associated value should look like:
{
'n': ..., # e.g., 4
'return value': ..., # e.g., '24'
'called by': ..., # e.g., 'fact(5)'
'call string': ... # e.g., 'fact(4)'
}That is, move some of the logic in the get_recursive_dict method into the _record_call method to provide more structure to the data at the time when the call is initially recorded.
Finally, note that I've asked you to store 'call string'; if you do so, you should be able to look up previously-computed call strings rather than recomputing them for every "parent" call string.
python_ta/debug/recursion_table.py
Outdated
| """ | ||
|
|
||
| frames_mapping: dict[types.FrameType, list] | ||
| frames_ordered: list[types.FrameType] |
There was a problem hiding this comment.
This attribute is not necessary and can be removed. You've correctly observed elsewhere that dictionaries preserve insertion order, so you can recover the original (pre-order) function call order by iterating over the previous instance attribute.
If you want to access the first key from a dictionary, you can use next(my_dict).
python_ta/debug/recursion_table.py
Outdated
| """Update the state of the table representation after a function return is detected. | ||
| Note: the frame must already have been seen as returns are done 'on the way out'. | ||
| """ | ||
| self.frames_data[frame]["return value"] = return_value |
There was a problem hiding this comment.
You'll probably want a deepcopy here as well
Motivation and Context
This pull request contains a new feature for recursive tracing similar to AccumulationTable. It outputs a table of the inputs, return value, and caller inputs in order of the recursive calls. The table also maintains a tree representation of the recursive call which can be used for visualization purposes.
Your Changes
Description:
I created a new file for the RecursionTable class, one for the Tree class, and various tests to ensure the recursive table and corresponding tree structure are correct.
Type of change:
Testing
I made an extensive test suite for various types of recursive functions and made sure to verify both the outputted table and the tree structure created. I also did manual testing in a playground to ensure the outputs and visualizations were as expected.
Checklist