Skip to content

Conversation

@kovan
Copy link

@kovan kovan commented Feb 3, 2026

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>
Copy link
Member

@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.

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
Copy link

bedevere-app bot commented Feb 3, 2026

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
Copy link
Author

kovan commented Feb 4, 2026

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
Copy link
Member

picnixz commented Feb 4, 2026

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 picnixz closed this Feb 4, 2026
@kovan
Copy link
Author

kovan commented Feb 4, 2026

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

@kovan
Copy link
Author

kovan commented Feb 4, 2026

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
Copy link
Member

picnixz commented Feb 4, 2026

No, debate is in the issue.

@picnixz
Copy link
Member

picnixz commented Feb 4, 2026

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
Copy link
Author

kovan commented Feb 4, 2026

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

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