bpo-30419: DOC: Update missing information in bdb docs #1687
Conversation
|
@csabella, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tiran, @benjaminp and @birkenfeld to be potential reviewers. |
|
I'm sorry but it seems that you mentionned the wrong bpo number. |
csabella
changed the title
|
Sorry about that, it was 30419. |
Doc/library/bdb.rst
Outdated
| For real file names, the canonical form is an operating system dependent, | ||
| :func:`case-normalized <os.path.normcase>` :func:`absolute path | ||
| <os.path.abspath>`. A *filename* with angle brackets, such as `"<stdin>"` | ||
| generated in interactive mode, are returned unchanged. |
There was a problem hiding this comment.
"A filename ... is" (not are) or "Filenames .... are"
There was a problem hiding this comment.
Thanks for catching that!
Doc/library/bdb.rst
Outdated
| matching breakpoint. | ||
| Return a tuple of the breakpoint that was triggered and a boolean that | ||
| indicates if it is ok to delete a temporary breakpoint. Return | ||
| ``(None, None)`` if there is no matching breakpoint. |
There was a problem hiding this comment.
I want to put this in the standard format. My suggestion:
"""Return (active (file, line) breakpoint, delete temporary flag) or (None, None).
The breakpoint is the first entry in Breakpoint.bplist[file, line] (which must exist) that is enabled and has neither a false condition nor positive ignore count. The flag, meaning that a temporary breakpoint should be deleted, is False only when the condition cannot be evaluated (in which case, ignore count is ignored).
"""
What do you think? The docstring for effective() should also be replaced and the obsolete pair of comment lines above the def line deleted.
There was a problem hiding this comment.
I've made these changes. I also made other changes to add all the attributes for Breakpoint and then use those as links in this description of effective. Sorry to add so much to the second review, but I think it helps a lot in understanding the breakpoints. One thing that I didn't say is that ignore would be set by the caller. Not sure if I need to mention that it's a read/write attribute?
I’m not familiar with this particular module, but I found a couple places with apparently unfinished sentences (or at that least don’t make grammatical sense to me).
Doc/library/bdb.rst
Outdated
| .. attribute:: line | ||
|
|
||
| Line number of the :class:`Breakpoint` within | ||
| :attr:`file <bdb.Breakpoint.file>`. |
There was a problem hiding this comment.
Since this is all indented under class:: Breakpoint, I think you should be able to just link with :attr:`file`. (Failing that, there is another shortcut: :attr:`~bdb.Breakpoint.file`.)
Doc/library/bdb.rst
Outdated
|
|
||
| .. attribute:: temporary | ||
|
|
||
| True if :class:`Breakpoint` at the (file, line) is temporary. |
There was a problem hiding this comment.
I’m not familiar with this particular module, but it seems like these would read better if you dropped the, so they are just at (file, line). Or do you need to move the before Breakpoint?
There was a problem hiding this comment.
Thanks, I added an 'a' before Breakpoint.
Doc/library/bdb.rst
Outdated
| of surrounding angle brackets. | ||
| Return canonical form of *filename*. | ||
|
|
||
| For real file names, the canonical form is an operating system dependent, |
There was a problem hiding this comment.
operating-system-dependent (dependent is an adjective, not a noun here)
Doc/library/bdb.rst
Outdated
| @@ -311,16 +361,18 @@ The :mod:`bdb` module also defines two classes: | |||
|
|
|||
| .. method:: get_stack(f, t) | |||
|
|
|||
| Get a list of records for a frame and all higher (calling) and lower | |||
| frames, and the size of the higher part. | |||
| Return a list of (frame, lineno) tuples in a stack trace and a size. | |||
There was a problem hiding this comment.
Does this need a comma before and a size? Are the tuples in the size or just in the stack trace?
There was a problem hiding this comment.
The output is ((stack trace), size). So, it's a tuple of the form (list of tuples, int). I've added the comma before "and a size".
|
To try and help move older pull requests forward, we are going through and backfilling 'awaiting' labels on pull requests that are lacking the label. Based on the current reviews, the best we can tell in an automated fashion is that a core developer requested changes to be made to this pull request. If/when the requested changes have been made, please leave a comment that says, |
This PR has had quite the journey. I think it's mostly a good improvement, but I have a few comments.
Co-authored-by: Martin Panter <[email protected]>
Update missing information identified when creating docstrings.
#74604