FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-140771: Add class directive for xmlparser in pyexpat docs by kovan · Pull Request #144456 · python/cpython · GitHub

Repository navigation

gh-140771: Add class directive for xmlparser in pyexpat docs - #144456

Closed
kovan wants to merge 1 commit into
python:mainfrom
kovan:gh-140771-pyexpat-xmlparsertype
Closed

kovan wants to merge 1 commit into
python:mainfrom
kovan:gh-140771-pyexpat-xmlparsertype

Conversation

kovan commented Feb 3, 2026 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Adds a .. class:: xmlparser directive so that all :class:xmlparser`` references in the documentation resolve properly
  • Documents that the type is available as xml.parsers.expat.XMLParserType
  • Updates XMLParserType to be documented as an alias for the class

This fixes the broken cross-references throughout the pyexpat documentation.

Test plan

  • make check passed
  • Documentation builds correctly

🤖 Generated with Claude Code


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

Add a class directive for xmlparser so that all the :class:`xmlparser`
references in the documentation resolve properly. Document that the
type is available as xml.parsers.expat.XMLParserType.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

picnixz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

xmlparser does not exist at runtime so we can't document it here. We need an another way to make references work but that may require a Sphinx change.

bedevere-app Bot commented Feb 3, 2026

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.

kovan commented Feb 4, 2026

Copy link
Copy Markdown
Contributor Author

The class does exist at runtime - it's just accessed via XMLParserType rather than directly:

>>> import xml.parsers.expat as expat
>>> expat.XMLParserType
<class 'pyexpat.xmlparser'>
>>> expat.XMLParserType.__name__
'xmlparser'
>>> type(expat.ParserCreate())
<class 'pyexpat.xmlparser'>

The documentation already uses xmlparser in 30+ places (method signatures like xmlparser.Parse(), cross-references like :class:xmlparser``). Adding .. class:: xmlparser creates the anchor needed for those references to resolve.

The note in my PR already clarifies the access pattern:

"This type is available as xml.parsers.expat.XMLParserType."

Would this approach be acceptable, or do you prefer changing all 30+ references from xmlparser to XMLParserType instead?

picnixz commented Feb 4, 2026

Copy link
Copy Markdown
Member

The class does exist at runtime - it's just accessed via XMLParserType rather than directly:

The name does not exist. We don't document objects that don't exist at runtime. You can't do pyexpat.xmlparser.

Would this approach be acceptable, or do you prefer changing all 30+ references from xmlparser to XMLParserType instead?

I want to think about it differently. Ideally, I'd like to have a real xmlparser at runtime but it's... weird. I also don't want to break Sphinx inventories or external links, but I also don't want to change all refs (that's what I did first). So for now, I prefer that we have a clear solution before opening PRs and thus will close this PR. In general, don't open PRs if there is no consensus on the issue or a clear solution.

picnixz closed this Feb 4, 2026

kovan commented Feb 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

The thing is, I am an action man, you know. And Python has 5000+! open issues, so plenty of room to work here.

kovan commented Feb 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Also, the debate can be in the PRs, that's fine for me. I can make edits on the PRs as the conversation evolves.

picnixz commented Feb 4, 2026

Copy link
Copy Markdown
Member

No, debate is in the issue.

picnixz commented Feb 4, 2026

Copy link
Copy Markdown
Member

The thing is, I am an action man, you know. And Python has 5000+! open issues, so plenty of room to work here.

Yes, but you need to be careful. If you just generate PRs with AI, we won't accept them because it takes time to review them. I prefer having 5k+ issues open rather than lots of low-quality PRs generated by an LLM.

kovan commented Feb 4, 2026

Copy link
Copy Markdown
Contributor Author

Yeah, but 5000+ open issues is just not acceptable.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting changes docs Documentation in the Doc dir skip news

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL