GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-125436: add missing allow_unnamed_section doc; split too long line by litlighilit · Pull Request #125437 · python/cpython

Conversation

@litlighilit

@litlighilit litlighilit commented Oct 14, 2024 •
edited by github-actions Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • Issue: Doc for ConfigParser misses allow_unnamed_section=False param #125436

📚 Documentation preview 📚: https://cpython-previews--125437.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
litlighilit added 2 commits October 14, 2024 16:23
@litlighilit
pythongh-125436: add missing allow_unnamed_section doc
58ffc51
@litlighilit
pythongh-125436: split too long line of ConfigParser doc
4f84f43
@bedevere-app bedevere-app Bot added docs Documentation in the Doc dir skip news awaiting review labels Oct 14, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Oct 14, 2024
Doc for ConfigParser misses allow_unnamed_section=False param #125436
Closed
@Eclips4 Eclips4 added the needs backport to 3.13 only security fixes label Oct 14, 2024
Eclips4
Eclips4 requested changes Oct 14, 2024
View reviewed changes

@Eclips4 Eclips4 left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Please add a versionchanged section for the allow_unnamed_section parameter.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot removed the awaiting review label Oct 14, 2024
@bedevere-app

bedevere-app Bot commented Oct 14, 2024

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added the awaiting changes label Oct 14, 2024
@Eclips4

Eclips4 commented Oct 14, 2024

Copy link
Copy Markdown
Member

Though, let' not add a new versionchanged section, let's add a versionadded section with a short explanation of what allow_unnamed_section is doing.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@litlighilit

litlighilit commented Oct 14, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Many params own their decription below function doc.

So is ConfigParser's default_section converter param.

Screenshot_2024-10-14-17-05-44-337_org.mozilla.firefox-edit.jpg

a short explanation of what allow_unnamed_section is doing.

Shall allow_unnamed_section's decription go here, too?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Eclips4

Eclips4 commented Oct 14, 2024

Copy link
Copy Markdown
Member

Shall allow_unnamed_section's decription go here, too?

Yes, it should. After its description there should be a versionadded directive.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@picnixz

picnixz commented Oct 14, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

You may also add the versionadded after the doctests, where the other versionchanged were put (just be careful to order it by version).
Eclips4 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@litlighilit
pythongh-125436: fixup missing desc and verionchanged
b8db50b
@litlighilit

litlighilit commented Oct 14, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Both of desc and versionchanged are done.

See if the added desc for allow_unnamed_section is suitable.

I've seen the CI failure but not sure what's up.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vadmium
vadmium reviewed Oct 14, 2024
View reviewed changes
Comment thread Doc/library/configparser.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/configparser.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@litlighilit
pythongh-125436: add *; apply suggestion from @picnixz
5dea065 
- add space in ref before `<` char of URL to fix compile error;
- add `*` to mark keywords (fixup) of ConfigParser;
- simpify desc for `allow_unnamed_section` param;
@litlighilit
litlighilit requested a review from Eclips4 October 15, 2024 01:08
Eclips4
Eclips4 requested changes Oct 17, 2024
View reviewed changes

@Eclips4 Eclips4 left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks for the changes!
Taking a closer look at the #117273 I've noticed that the class ConfigParser inherits from the RawConfigParser and therefore parameter allow_unnamed_section has actually been added to the RawConfigParser class. Therefore, we need to do two things:

  • Update doc for ConfigParser (which you have already done!)
  • Update doc for RawConfigParser. Although this class is considered legacy in docs, it would still be useful to update its signature. The section with an explanation what allow_unnamed_section is doing unnecessary here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
litlighilit reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@litlighilit
pythongh-125436: update signature of RawConfigParser...
673a26c 
- apply suggestion from python#125437 (review)
- also update for  `versionchanged` for params of RawConfigParser
@litlighilit
litlighilit requested a review from Eclips4 October 17, 2024 17:42
@litlighilit

litlighilit commented Oct 21, 2024

Copy link
Copy Markdown
Contributor Author

ping @Eclips4
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Eclips4
Merge branch 'main' into lit-fix-doc
79e34d2
Eclips4
Eclips4 approved these changes Nov 5, 2024
View reviewed changes

@Eclips4 Eclips4 left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry for waiting @litlighilit! LGTM. Thank you!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting changes labels Nov 5, 2024
@Eclips4
Eclips4 enabled auto-merge (squash) November 5, 2024 02:34
@Eclips4
Eclips4 merged commit d960226 into python:main Nov 5, 2024
@miss-islington-app

miss-islington-app Bot commented Nov 5, 2024

Copy link
Copy Markdown

Thanks @litlighilit for the PR, and @Eclips4 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label Nov 5, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 5, 2024
@litlighilit @miss-islington
pythongh-125436: Doc: Add missing allow_unnamed_section parameter…
c41f0d9 
… to ``ConfigParser`` documentation (pythonGH-125437)

Add missing ``allow_unnamed_section`` parameter to ``ConfigParser`` doc, as well as to it's parent ``RawConfigParser``.
Split too long line on ``ConfigParser`` signature.
Add some sections about when some of ``RawConfigParser`` parameters were added.
(cherry picked from commit d960226)

Co-authored-by: lit <[email protected]>
@bedevere-app

bedevere-app Bot commented Nov 5, 2024

Copy link
Copy Markdown

GH-126421 is a backport of this pull request to the 3.13 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Nov 5, 2024
Eclips4 pushed a commit that referenced this pull request Nov 5, 2024
@miss-islington @litlighilit
[3.13] gh-125436: Doc: Add missing allow_unnamed_section paramete…
05da4ee 
…r to ``ConfigParser`` documentation (GH-125437) (#126421)

gh-125436: Doc: Add missing ``allow_unnamed_section`` parameter to ``ConfigParser`` documentation (GH-125437)

Add missing ``allow_unnamed_section`` parameter to ``ConfigParser`` doc, as well as to it's parent ``RawConfigParser``.
Split too long line on ``ConfigParser`` signature.
Add some sections about when some of ``RawConfigParser`` parameters were added.
(cherry picked from commit d960226)

Co-authored-by: lit <[email protected]>
picnixz pushed a commit to picnixz/cpython that referenced this pull request Dec 8, 2024
@litlighilit @picnixz
pythongh-125436: Doc: Add missing allow_unnamed_section parameter…
316e6a2 
… to ``ConfigParser`` documentation (python#125437)

Add missing ``allow_unnamed_section`` parameter to ``ConfigParser`` doc, as well as to it's parent ``RawConfigParser``.
Split too long line on ``ConfigParser`` signature.
Add some sections about when some of ``RawConfigParser`` parameters were added.
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
@litlighilit @ebonnal
pythongh-125436: Doc: Add missing allow_unnamed_section parameter…
78d61e2 
… to ``ConfigParser`` documentation (python#125437)

Add missing ``allow_unnamed_section`` parameter to ``ConfigParser`` doc, as well as to it's parent ``RawConfigParser``.
Split too long line on ``ConfigParser`` signature.
Add some sections about when some of ``RawConfigParser`` parameters were added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vadmium vadmium vadmium left review comments
@Eclips4 Eclips4 Eclips4 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news

Projects

Docs PRs
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@litlighilit @Eclips4 @picnixz @vadmium
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.