Skip to content

Conversation

@picnixz
Copy link
Member

@picnixz picnixz commented May 16, 2025

string.Formatter is using legacy docstrings that are picked up by pydoc but are not by modern IDEs. I suggest changing them to regular docstrings.

@picnixz picnixz requested a review from serhiy-storchaka May 16, 2025 12:01
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

@picnixz picnixz enabled auto-merge (squash) May 16, 2025 18:05
@picnixz picnixz merged commit b5febf7 into python:main May 16, 2025
38 checks passed
@picnixz picnixz deleted the feat/string/reST-docs-134082 branch May 17, 2025 07:49
obj = getattr(obj, i)
else:
obj = obj[i]

Copy link
Member

Choose a reason for hiding this comment

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

why remove empty lines that provide «breathing space» and make it easy to follow the code?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mmh, I shouldn't have removed those ones indeed. My bad. I however think that too many empty lines hinder the flow but I should have left them out in this case. I'll try to keep in mind. Note that from what I saw in more modern code, we tend to avoid too many blank lines in general except in long functions where logical sections are split.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @picnixz. PEP 8 says to use a single blank line to separate methods and two blank lines to separate top-level functions and classes (there are also exceptions). https://peps.python.org/pep-0008/#blank-lines

We don't usually make changes solely to meet PEP 8 requirements, but in this case I think it was justified.

Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
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.

3 participants