Skip to content

bpo-30211: bdb: add docstrings #1350

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.

Sign up for GitHub

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

Merged
terryjreedy merged 9 commits into from
May 16, 2017
Merged

bpo-30211: bdb: add docstrings #1350

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3adafa2
bpo-19417: bdb: add docstrings
csabella Apr 28, 2017
c335e37
bpo-19417: bdb: add docstrings
csabella Apr 29, 2017
f898ce1
bpo-30211: bdb: add docstrings
csabella May 8, 2017
7f7a1d1
bpo-30211: bdb: add docstrings
csabella May 8, 2017
7805f71
Merge remote-tracking branch 'origin/master' into pr_1350
terryjreedy May 16, 2017
5d8a9e7
Revise proposed docstrings. Some changes are reformatting to 72 chars.
terryjreedy May 16, 2017
d43ad02
Fix typos
terryjreedy May 16, 2017
8d88214
Fix typos
terryjreedy May 16, 2017
7096c82
Merge remote-tracking branch 'origin' into pr_1350
terryjreedy May 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
bpo-19417: bdb: add docstrings
  • Loading branch information
@csabella
csabella committed Apr 29, 2017
commit c335e37f85f6efb7bee596072c26098ed11dc85d
56 changes: 31 additions & 25 deletions Lib/bdb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ class Bdb:
that originate in a module that matches one of these patterns.
Whether a frame is considered to originate in a certain module
is determined by the __name__ in the frame globals.

"""

def __init__(self, skip=None):
Expand All @@ -34,11 +33,10 @@ def __init__(self, skip=None):
self.frame_returning = None

def canonic(self, filename):
"""Get a filename into a canonical form.
"""Return canonical form of filename.

A canonical form is a case-normalized (on case insenstive filesystems)
absolute path, stripped of surrounding angle brackets.

"""
if filename == "<" + filename[1:-1] + ">":
return filename
Expand All @@ -57,9 +55,10 @@ def reset(self):
self._set_stopinfo(None, None)

def trace_dispatch(self, frame, event, arg):
"""Trace function of debugged frames.
"""Dispatch a trace function for debugged frames based on the event.

The return value is the new trace function, which is usually itself.
This function is installed as the trace function for debugged frames.
Its return value is the new trace function, which is usually itself.
The default implementation decides how to dispatch a frame, depending
on the type of event (passed in as a string) that is about to be
executed.
Expand All @@ -82,7 +81,6 @@ def trace_dispatch(self, frame, event, arg):
See sys.settrace() for more information on the trace function. For
more information on code and frame objects, refer to The Standard
Type Hierarchy.

"""
if self.quitting:
return # None
Expand All @@ -104,27 +102,25 @@ def trace_dispatch(self, frame, event, arg):
return self.trace_dispatch

def dispatch_line(self, frame):
"""Trace function when a new line of code is executed.
"""Invoke user function and return trace function for line event.

If the debugger stops on the current line, invoke the user_line()
method. Raise a BdbQuit exception if the Bdb.quitting flag is set.
Return a reference to the trace_dispatch() method for further
tracing in that scope.

"""
if self.stop_here(frame) or self.break_here(frame):
self.user_line(frame)
if self.quitting: raise BdbQuit
return self.trace_dispatch

def dispatch_call(self, frame, arg):
"""Trace function when a function is called or code block entered.
"""Invoke user function and return trace function for call event.

If the debugger stops on this function call, invoke the user_call()
method. Raise a BbdQuit exception if the Bdb.quitting flag is set.
Return a reference to the trace_dispatch() method for further
tracing in that scope.

"""
# XXX 'arg' is no longer used
if self.botframe is None:
Expand All @@ -142,13 +138,12 @@ def dispatch_call(self, frame, arg):
return self.trace_dispatch

def dispatch_return(self, frame, arg):
"""Trace function when a function or code block is about to return.
"""Invoke user function and return trace function for return event.

If the debugger stops on this function return, invoke the user_return()
method. Raise a BdbQuit exception if the Bdb.quitting flag is set.
Return a reference to the trace_dispatch() method for further
tracing in that scope.

"""
if self.stop_here(frame) or frame == self.returnframe:
# Ignore return events in generator except when stepping.
Expand All @@ -166,13 +161,12 @@ def dispatch_return(self, frame, arg):
return self.trace_dispatch

def dispatch_exception(self, frame, arg):
"""Trace function when an exception has occurred.
"""Invoke user function and return trace function for exception event.

If the debugger stops on this function return, invoke the
user_exception() method. Raise a BdbQuit exception if the
Bdb.quitting flag is set. Return a reference to the trace_dispatch()

@marco-buttu marco-buttu May 8, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is a missing whitespace after the period.

@csabella csabella May 8, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Corrected.

method for further tracing in that scope.

"""
if self.stop_here(frame):
# When stepping with next/until/return in a generator frame, skip
Expand Down Expand Up @@ -226,7 +220,6 @@ def break_here(self, frame):
Check if there is a breakpoint in the filename and lineno
belonging to the frame or in the current function. If the breakpoint
is a temporary one, delete it.

"""
filename = self.canonic(frame.f_code.co_filename)
if filename not in self.breaks:
Expand Down Expand Up @@ -284,6 +277,12 @@ def user_exception(self, frame, exc_info):
pass

def _set_stopinfo(self, stopframe, returnframe, stoplineno=0):
"""Set the attributes for stopping.

If `stoplineno` is greater than or equal to 0, then stop at line

@marco-buttu marco-buttu May 8, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In this doctstring you are wrapping the argument stoplineno with `, while in all the other doctstrings you are not. I think to be consistent you have to remove the `` around stoplineno.

@csabella csabella May 8, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I've removed the backticks on the ones I added. There is still one set of backticks that I didn't add. Do you think I should remove those too?

@marco-buttu marco-buttu May 8, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we can also remove the backticks in checkfuncname(), so we'll have all the docstrings formatted consistently.

@csabella csabella May 8, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. I've also changed checkfuncname() to match the doc page.

greater than or equal to the stopline. If `stoplineno` is -1, then
don't stop at all.
"""
self.stopframe = stopframe
self.returnframe = returnframe
self.quitting = False
Expand Down Expand Up @@ -376,8 +375,8 @@ def set_break(self, filename, lineno, temporary=False, cond=None,
funcname=None):
"""Set a new breakpoint for the filename:lineno.

If lineno doesn't exist for the filename, return error. The
filename should be in canonical form.
If lineno doesn't exist for the filename, return an error message.
The filename should be in canonical form.
"""
filename = self.canonic(filename)
import linecache # Import as late as possible
Expand All @@ -388,8 +387,16 @@ def set_break(self, filename, lineno, temporary=False, cond=None,
if lineno not in list:
list.append(lineno)
bp = Breakpoint(filename, lineno, temporary, cond, funcname)
return None

def _prune_breaks(self, filename, lineno):
"""Prune breakpoints from filname:lineno.

A list of breakpoints is maintained in the Bdb instance and in
the Breakpoint class. If a breakpoint in the Bdb instance no
longer exists in the Breakpoint class, then it's removed from the
Bdb instance.
"""
if (filename, lineno) not in Breakpoint.bplist:
self.breaks[filename].remove(lineno)
if not self.breaks[filename]:
Expand All @@ -398,7 +405,7 @@ def _prune_breaks(self, filename, lineno):
def clear_break(self, filename, lineno):
"""Delete breakpoints in filename:lineno.

If none were set, return an error message.
If no breakpoints were set, return an error message.
"""
filename = self.canonic(filename)
if filename not in self.breaks:
Expand All @@ -410,6 +417,7 @@ def clear_break(self, filename, lineno):
for bp in Breakpoint.bplist[filename, lineno][:]:
bp.deleteMe()
self._prune_breaks(filename, lineno)
return None

def clear_bpbynumber(self, arg):
"""Delete a breakpoint by its index in Breakpoint.bpbynumber.
Expand All @@ -422,6 +430,7 @@ def clear_bpbynumber(self, arg):
return str(err)
bp.deleteMe()
self._prune_breaks(bp.file, bp.line)
return None

def clear_all_file_breaks(self, filename):
"""Delete all breakpoints in filename.
Expand All @@ -436,6 +445,7 @@ def clear_all_file_breaks(self, filename):
for bp in blist:
bp.deleteMe()
del self.breaks[filename]

@terryjreedy terryjreedy Apr 29, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When there is an explicit non-None return in a function, then any None return should be explicit. (This is somewhere in PEP 8.) So add 'return None' on new line after this.

@csabella csabella Apr 29, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I hadn't changed any code, so I wasn't sure about this when I saw it. I've now added None to all the similar set_* functions that had another return and no return None. Question -- if there are no returns, should I add return None?

Another question: there is some code like this - "if self.quitting: raise BdbQuit". Should I put that on two lines?

@terryjreedy terryjreedy Apr 29, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Clarification: add 'return None' at the end of the function ;-). I missed 'set_break' and any others because of the folding; thank you for checking. The relevant PEP8 section begins "Be consistent in return statements." It is standard to omit 'return None' at the end and leave it implicit when there are no other return statements.

For 'if cond: statement', PEP8 says both 'Rather not:' and 'While sometimes it's okay'. I think we should leave them alone.

@csabella csabella Apr 29, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.

return None

def clear_all_breaks(self):
"""Delete all existing breakpoints.
Expand All @@ -448,6 +458,7 @@ def clear_all_breaks(self):
if bp:
bp.deleteMe()
self.breaks = {}
return None

def get_bpbynumber(self, arg):
"""Return a breakpoint by its index in Breakpoint.bybpnumber.
Expand Down Expand Up @@ -478,7 +489,7 @@ def get_break(self, filename, lineno):
def get_breaks(self, filename, lineno):
"""Return all breakpoints for filename:lineno.

If no breakpoints are set, returns an empty list.
If no breakpoints are set, return an empty list.
"""
filename = self.canonic(filename)
return filename in self.breaks and \
Expand All @@ -488,7 +499,7 @@ def get_breaks(self, filename, lineno):
def get_file_breaks(self, filename):
"""Return all lines with breakpoints for filename.

If no breakpoints are set, returns an empty list.
If no breakpoints are set, return an empty list.
"""
filename = self.canonic(filename)
if filename in self.breaks:
Expand Down Expand Up @@ -652,7 +663,6 @@ class Breakpoint:
in canonical form. If `funcname` is defined, a breakpoint hit will be
counted when the first line of that function is executed. A
conditional breakpoint always counts a hit.

"""

# XXX Keeping state in the class is a mistake -- this means
Expand Down Expand Up @@ -689,7 +699,6 @@ def deleteMe(self):

If it is the last breakpoint in that position, it also deletes
the entry for the file:line.

"""

index = (self.file, self.line)
Expand All @@ -712,7 +721,6 @@ def bpprint(self, out=None):

The optional out argument directs where the output is sent
and defaults to standard output.

"""
if out is None:
out = sys.stdout
Expand All @@ -724,7 +732,6 @@ def bpformat(self):
The information is nicely formatted and includes the breakpoint number,
temporary status, file:line position, break condition, number of times
to ignore, and number of times hit.

"""
if self.temporary:
disp = 'del '
Expand Down Expand Up @@ -789,7 +796,6 @@ def effective(file, line, frame):
Called only if we know there is a bpt at this
location. Returns breakpoint that was triggered and a flag
that indicates if it is ok to delete a temporary bp.

"""
possibles = Breakpoint.bplist[file, line]
for b in possibles:
Expand Down