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

Jump to bottom

bpo-39278: add docstrings to functions in pdb module #17924

Merged
merged 1 commit into from May 10, 2022
Merged

bpo-39278: add docstrings to functions in pdb module #17924

merged 1 commit into from May 10, 2022

Conversation

carlbordum
Copy link
Contributor

@carlbordum carlbordum commented Jan 9, 2020 •
edited by bedevere-bot

All these functions were already documented, but not in docstrings which
means you could not call help() on the functions before this patch.

https://bugs.python.org/issue39278

@aeros aeros added the skip news label Jan 9, 2020
aeros
aeros requested changes Jan 9, 2020
View changes
Copy link
Member

@aeros aeros left a comment •
edited

Thanks for the PR @carlbordum. IMO, it's always helpful to keep the docstrings up to date with the documentation.

I've verified that the changes match the documentation at https://docs.python.org/3/library/pdb.html. The only thing that's missing is a few formatting conversions, particularly the words that should be italics. In docstrings, we typically surround parameter names with asterisks (only for the first occurrence in each docstring though). I'll include the missing ones in suggestions below.

The only other thing you could consider including would be a docstring for the Pdb class (https://docs.python.org/3/library/pdb.html#pdb.Pdb). But, the PR is still quite helpful in it's current state, so I think it could be merged with or without that addition once the asterisks are added.

Lib/pdb.py Outdated Show resolved Hide resolved
Lib/pdb.py Outdated Show resolved Hide resolved
Lib/pdb.py Outdated Show resolved Hide resolved
Lib/pdb.py Show resolved Hide resolved
Lib/pdb.py Show resolved Hide resolved
Lib/pdb.py Outdated Show resolved Hide resolved
Lib/pdb.py Outdated Show resolved Hide resolved
Lib/pdb.py Show resolved Hide resolved
@aeros
Copy link
Member

aeros commented Jan 12, 2020

@vstinner Based on the build logs at https://github.com/python/cpython/pull/17924/checks?check_run_id=382949957 and https://travis-ci.org/python/cpython/jobs/635143388?utm_medium=notification&utm_source=github_status, the failures look to be unrelated to the PR.

I suspect the "Install Dependencies" failure would likely be resolved if we re-triggered the CI checks, but the doc build failure in "library/nntplib.rst" might be a bit more involved:

Warning, treated as error:
**********************************************************************
File "library/nntplib.rst", line ?, in default
Failed example:
    s = NNTP('news.gmane.io')
Exception raised:
    Traceback (most recent call last):
      File "/home/travis/build/python/cpython/Lib/doctest.py", line 1329, in __run
        exec(compile(example.source, filename, "single",
      File "<doctest default[0]>", line 1, in <module>
        s = NNTP('news.gmane.io')
      File "/home/travis/build/python/cpython/Lib/nntplib.py", line 1050, in __init__
        _NNTPBase.__init__(self, file, host,
      File "/home/travis/build/python/cpython/Lib/nntplib.py", line 331, in __init__
        self.welcome = self._getresp()
      File "/home/travis/build/python/cpython/Lib/nntplib.py", line 456, in _getresp
        raise NNTPTemporaryError(resp)
    nntplib.NNTPTemporaryError: 400 load at 16.57, try later

Can we retrigger the tests here or would you suggest opening a bug report (if there's not one already)?

@vstinner
Copy link
Member

vstinner commented Jan 15, 2020

Failed example: s = NNTP('news.gmane.io')

I restarted the job. See https://bugs.python.org/issue39343

@carlbordum
bpo-39278: add docstrings to functions in pdb module
2dbb63b 
All these functions were already documented, but not in docstrings which
means you could not call help() on the functions before this patch.
@carlbordum
Copy link
Contributor Author

carlbordum commented May 10, 2022

It's been a while, huh 😅

I've rebased the commit and the tests now all pass.

@rhettinger rhettinger merged commit f481a02 into python:main May 10, 2022
12 checks passed
@vstinner
Copy link
Member

vstinner commented May 10, 2022

It's been a while, huh sweat_smile

Better later than never. Thanks!

@carlbordum carlbordum mannequin mentioned this pull request May 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aeros aeros

@rhettinger rhettinger

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@carlbordum @aeros @vstinner @rhettinger @the-knights-who-say-ni @bedevere-bot