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.

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

Jump to bottom

gh-122058: Lib/inspect: Update docstrings for isfunction, isgenerator, isframe, iscode. #122059

Merged
merged 11 commits into from
Aug 6, 2024
Merged

gh-122058: Lib/inspect: Update docstrings for isfunction, isgenerator, isframe, iscode. #122059

merged 11 commits into from
Aug 6, 2024

Conversation

Eclips4
Copy link
Member

@Eclips4 Eclips4 commented Jul 20, 2024 •
edited by bedevere-app bot
Loading

@Eclips4 Eclips4 added skip news needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes labels Jul 20, 2024
@bedevere-app bedevere-app bot mentioned this pull request Jul 20, 2024
Closed
@Eclips4 Eclips4 requested a review from vstinner July 20, 2024 13:34
AA-Turner
AA-Turner reviewed Jul 20, 2024
View reviewed changes
Lib/inspect.py Outdated Show resolved Hide resolved
picnixz
picnixz reviewed Jul 20, 2024
View reviewed changes
Copy link
Contributor

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

I would suggest synchronizing what is being written in the docstring with what is being written in inspect.rst and have the same wording.

Lib/inspect.py Outdated Show resolved Hide resolved
Lib/inspect.py Outdated Show resolved Hide resolved
Lib/inspect.py Show resolved Hide resolved
picnixz
picnixz reviewed Jul 27, 2024
View reviewed changes
Lib/inspect.py Outdated Show resolved Hide resolved
Lib/inspect.py Outdated Show resolved Hide resolved
Doc/library/inspect.rst Outdated Show resolved Hide resolved
Doc/library/inspect.rst Outdated Show resolved Hide resolved
Doc/library/inspect.rst Outdated Show resolved Hide resolved
Eclips4 and others added 4 commits July 27, 2024 11:40
@Eclips4 @picnixz
Update Lib/inspect.py
506da65 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@Eclips4 @picnixz
Update Lib/inspect.py
c5966f6 
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@Eclips4
Address review
d6b3439
@Eclips4
Fix linting issue
08e4b6b
@Eclips4 Eclips4 requested a review from picnixz July 27, 2024 08:50
picnixz
picnixz approved these changes Jul 27, 2024
View reviewed changes
Copy link
Contributor

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

Looks good to me now. I'll let @AA-Turner review one last time in case I missed English typos.

vstinner
vstinner reviewed Jul 27, 2024
View reviewed changes
Doc/library/inspect.rst Outdated Show resolved Hide resolved
Doc/library/inspect.rst Outdated Show resolved Hide resolved
Doc/library/inspect.rst Show resolved Hide resolved
Lib/inspect.py Outdated Show resolved Hide resolved
Eclips4 and others added 3 commits July 27, 2024 23:49
Eclips4
Eclips4 commented Jul 27, 2024
View reviewed changes
Lib/inspect.py
send resumes the generator and "sends" a value that becomes
gi_yieldfrom object being iterated by yield from or None

__iter__() defined to support iteration over container
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI: I've grouped methods, but I'm not sure if __iter__ really needs to be here. If __iter__ is here, then we also need to add __next__. Our inspect documentation does not mention the __next__ method for generators. In my opinion, we should remove this method from the docstring since it does not make things clearer.

vstinner
vstinner approved these changes Jul 29, 2024
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

@vstinner
Copy link
Member

About backports, backporting inspect.rst changes LGTM, but I'm not sure about changing docstrings. Maybe docstrings should be left unchanged in 3.12 and 3.13 branches, especially in the 3.12 branch.

@vstinner vstinner removed needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes labels Aug 6, 2024
@vstinner vstinner merged commit 8ce70d6 into python:main Aug 6, 2024
41 checks passed
@vstinner
Copy link
Member

vstinner commented Aug 6, 2024

Merged, thanks.

I removed automated backported. As I wrote previously, if you want, you can backport .rst changes, but not .c files (docstrings), in stable branches.

@Eclips4 Eclips4 deleted the update-docstring branch August 6, 2024 17:04
brandtbucher pushed a commit to brandtbucher/cpython that referenced this pull request Aug 7, 2024
@Eclips4 @picnixz
@vstinner @brandtbucher
pythongh-122058: Lib/inspect: Update docstrings for isfunction, `…
3df4ace 
…isgenerator`, `isframe`, `iscode`. (python#122059)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
blhsing pushed a commit to blhsing/cpython that referenced this pull request Aug 22, 2024
@Eclips4 @picnixz
@vstinner @blhsing
pythongh-122058: Lib/inspect: Update docstrings for isfunction, `…
5d455ea 
…isgenerator`, `isframe`, `iscode`. (python#122059)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner approved these changes

@picnixz picnixz picnixz approved these changes

@AA-Turner AA-Turner Awaiting requested review from AA-Turner

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

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

Successfully merging this pull request may close these issues.
4 participants
@Eclips4 @vstinner @AA-Turner @picnixz