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

Preserve changes to frame locals when going up/down in the stack frame #13051

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ccordoba12
Copy link
Member

Description

This complements the excellent improvements on PR #13021 by preserving frame locals when going up/down in the stack. Below I show gifs of the previous and new behaviors using Spyder:

Before

Old

After

New

Implementation

This was accomplished by saving a copy of all frame locals before the debugger starts and using them to set curframe_locals after every command run in the debugger. That prevents that accesses to f_locals (which happens in up/down) reset the locals set by the user.

Notes

  • I draw inspiration for this from the patch present in https://bugs.python.org/issue9633, although it's very different.
  • There's PEP 558 that should solve this problem the right way, but currently is marked for Python 3.11 and an implementation for it has been four years in the making.

@ccordoba12 ccordoba12 force-pushed the preserve-frame-locals-on-up-down branch from ab1ac90 to 1337bbf Compare July 11, 2021 03:07
@ccordoba12 ccordoba12 requested a review from Carreau July 11, 2021 03:16
Copy link
Member

@MrMino MrMino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this causes a regression - it makes it impossible to change the values of the variables in the local scope. The changes are only visible to the debugger, but not to the debugged code.

On master branch:

Python 3.8.6 (default, Jun 23 2021, 23:50:42) 
Type 'copyright', 'credits' or 'license' for more information
IPython 8.0.0.dev -- An enhanced Interactive Python. Type '?' for help.

In [1]: def print_my_var():
   ...:     my_var = '==== unchanged ===='
   ...:     print(my_var)
   ...: 

In [2]: %debug print_my_var()
NOTE: Enter 'c' at the ipdb>  prompt to continue execution.
> <string>(1)<module>()

ipdb> s
--Call--
> <ipython-input-1-5aa28ff21139>(1)print_my_var()
----> 1 def print_my_var():
      2     my_var = '==== unchanged ===='
      3     print(my_var)
      4 

ipdb> n
> <ipython-input-1-5aa28ff21139>(2)print_my_var()
      1 def print_my_var():
----> 2     my_var = '==== unchanged ===='
      3     print(my_var)
      4 

ipdb> n
> <ipython-input-1-5aa28ff21139>(3)print_my_var()
      1 def print_my_var():
      2     my_var = '==== unchanged ===='
----> 3     print(my_var)
      4 

ipdb> my_var = "==== changed ===="
ipdb> n
==== changed ====
--Return--
None
> <ipython-input-1-5aa28ff21139>(3)print_my_var()
      1 def print_my_var():
      2     my_var = '==== unchanged ===='
----> 3     print(my_var)
      4 

With the changes presented in this PR:

Python 3.8.6 (default, Jun 23 2021, 23:50:42) 
Type 'copyright', 'credits' or 'license' for more information
IPython 8.0.0.dev -- An enhanced Interactive Python. Type '?' for help.

In [1]: def print_my_var():
   ...:     my_var = '==== unchanged ===='
   ...:     print(my_var)
   ...: 

In [2]: %debug print_my_var()
NOTE: Enter 'c' at the ipdb>  prompt to continue execution.
> <string>(1)<module>()

ipdb> s
--Call--
> <ipython-input-1-5aa28ff21139>(1)print_my_var()
----> 1 def print_my_var():
      2     my_var = '==== unchanged ===='
      3     print(my_var)
      4 

ipdb> n
> <ipython-input-1-5aa28ff21139>(2)print_my_var()
      1 def print_my_var():
----> 2     my_var = '==== unchanged ===='
      3     print(my_var)
      4 

ipdb> n
> <ipython-input-1-5aa28ff21139>(3)print_my_var()
      1 def print_my_var():
      2     my_var = '==== unchanged ===='
----> 3     print(my_var)
      4 

ipdb> my_var = "==== changed ===="
ipdb> n
==== unchanged ====
--Return--
None
> <ipython-input-1-5aa28ff21139>(3)print_my_var()
      1 def print_my_var():
      2     my_var = '==== unchanged ===='
----> 3     print(my_var)
      4 

@ccordoba12
Copy link
Member Author

Hey @MrMino, thanks a lot for checking that! I'll try to address that problem when I have some free time.

@Carreau Carreau added this to the 7.26 milestone Jul 13, 2021
@Carreau
Copy link
Member

Carreau commented Jul 13, 2021

+1 when this work. marking as for 7.x

@Carreau Carreau modified the milestones: 7.26, 7.27 Jul 31, 2021
@Carreau Carreau modified the milestones: 7.27, 7.28 Aug 27, 2021
when going up/down in the stack frame.
"""
self.frame_locals = [
stack_entry[0].f_locals.copy() for stack_entry in self.stack
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ccordoba12 , is there any need to copy this dict?
If there isn't, removing copy() could fix the problem mentioned by @MrMino (I've tested locally their example, and it worked as expected).

Suggested change
stack_entry[0].f_locals.copy() for stack_entry in self.stack
stack_entry[0].f_locals for stack_entry in self.stack

If you need to keep only certain subset of changes, I would suggest to wrap the original locals with a ChainMaps at some point.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you clarify? What's a ChainMap?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks a lot for the suggestion! I'll take a look at it as soon as Io have some free time.

@Carreau Carreau modified the milestones: 7.28, 7.29 Sep 25, 2021
@Carreau Carreau modified the milestones: 7.29, 7.30 Oct 29, 2021
@Carreau Carreau removed this from the 7.30 milestone Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants