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

Add set_trace to core debugger. #9947

Merged
merged 4 commits into from Sep 15, 2016
Merged

Add set_trace to core debugger. #9947

merged 4 commits into from Sep 15, 2016

Conversation

tillahoffmann
Copy link
Contributor

This PR adds a set_trace method to IPython.core.debugger to mirror the functionality of IPython.terminal.debugger. See #9940 for a discussion.

@@ -621,3 +621,14 @@ def do_where(self, arg):
self.print_stack_trace()

do_w = do_where


def set_trace(frame=None, condition=True):
Copy link
Member

Choose a reason for hiding this comment

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

Let's stick to the same API as pdb and IPython.terminal.debugger, with set_trace() taking no parameters. We can discuss adding parameters as a separate step, and if we do we'll want to keep core and terminal in sync.

@@ -621,3 +621,13 @@ def do_where(self, arg):
self.print_stack_trace()

do_w = do_where


def set_trace(frame=None):
Copy link
Member

Choose a reason for hiding this comment

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

The other set_trace() functions also don't have a frame parameter, though the set_trace() method does. I'd either remove it from here, or also add it to the version in IPython.terminal. @Carreau, opinions?

Also, if we decide to keep the parameter, it should probably actually do something ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it should indeed do something. Let me know if you'd prefer to remove the frame parameter or add it to IPython.terminal as well.

Copy link
Member

Choose a reason for hiding this comment

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

I could go either way, but I'd like to know what @Carreau thinks when he's online in a couple more hours. :-)

Copy link
Member

Choose a reason for hiding this comment

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

Happy to add it to the function, it should likely be passed to Pdb.set_trace() by adding 1 to it, and we should even likely start the debugging 1 frame up from the current one to be consistent with normal Pdb.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense, though it takes a frame object, not an offset number. Here's the set_trace definition from pdb:

def set_trace():
    Pdb().set_trace(sys._getframe().f_back)

Copy link
Member

@Carreau Carreau left a comment

Choose a reason for hiding this comment

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

Seem good to me ! (trying new GitHub reviews things)

@Carreau Carreau added this to the 5.2 milestone Sep 15, 2016
@Carreau
Copy link
Member

Carreau commented Sep 15, 2016

Marking as 5.2 as it should be easy to backport.

@takluyver
Copy link
Member

Looks good, thanks @tillahoffmann

@takluyver takluyver merged commit 185e47e into ipython:master Sep 15, 2016
@Carreau Carreau mentioned this pull request Sep 18, 2016
takluyver added a commit that referenced this pull request Jan 12, 2017
This PR adds a `set_trace` method to `IPython.core.debugger` to mirror the functionality of `IPython.terminal.debugger`. See  9940 for a discussion.

Signed-off-by: Thomas Kluyver <thomas@kluyver.me.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants