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

gh-85455: Add missing doc strings and improve docs #21573

Merged
merged 4 commits into from
Oct 15, 2022

Conversation

nanjekyejoannah
Copy link
Contributor

@nanjekyejoannah nanjekyejoannah commented Jul 21, 2020

Copy link
Member

@ammaraskar ammaraskar 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 and reads clearer.

Doc/library/imghdr.rst Outdated Show resolved Hide resolved
Copy link
Member

@facundobatista facundobatista left a comment

Choose a reason for hiding this comment

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

Thanks for this work! There I added some details regarding the docstrings, thanks!

Lib/imghdr.py Outdated
@@ -9,6 +9,9 @@
#-------------------------#

def what(file, h=None):
"""Return the type of image contained in a file or
byte stream.
"""
Copy link
Member

Choose a reason for hiding this comment

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

Please adapt PEP 257 in this docstring and the ones below.

In this case, just put everything in the same line and it will be ok.

Lib/imghdr.py Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@hugovk
Copy link
Member

hugovk commented Apr 11, 2022

Note the imghdr module is deprecated in 3.11 and set for removal in 3.13.

See PEP 594 – Removing dead batteries from the standard library and #91217.

@nanjekyejoannah
Copy link
Contributor Author

@iritkatriel and @hugovk So why is it wrong to add the changes, when the module is not removed yet? IIRC we are not yet on version 3.13, no?

@iritkatriel
Copy link
Member

It’s not wrong, it’s low priority.

Our intention is to reduce the PR backlog and sometimes that means to make a call that a low priority PR that nobody cared to merge for years is not worth investing core dev time in. Surely we won’t agree on each individual case. Feel free to reopen and merge this if you want to.

@nanjekyejoannah nanjekyejoannah changed the title bpo-41283: Add missing doc strings and improve docs gh-85455: Add missing doc strings and improve docs Apr 13, 2022
@hugovk
Copy link
Member

hugovk commented Apr 13, 2022

Yeah, this is a low-risk docs changes so I think it's fine to merge.

@nanjekyejoannah Would you like to address the changes suggested by Facundo?

@nanjekyejoannah
Copy link
Contributor Author

I have tracked this as a task for myself for now, I will do the needful and merge the PR in a day or two. Thanks.

@nanjekyejoannah nanjekyejoannah merged commit bf786e6 into python:main Oct 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants