| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
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>
There was a problem hiding this comment.
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.
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
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:
Would this approach be acceptable, or do you prefer changing all 30+ references from xmlparser to XMLParserType instead? |
Sorry, something went wrong.
The name does not exist. We don't document objects that don't exist at runtime. You can't do pyexpat.xmlparser.
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. |
Sorry, something went wrong.
|
The thing is, I am an action man, you know. And Python has 5000+! open issues, so plenty of room to work here. |
Sorry, something went wrong.
|
Also, the debate can be in the PRs, that's fine for me. I can make edits on the PRs as the conversation evolves. |
Sorry, something went wrong.
|
No, debate is in the issue. |
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
Yeah, but 5000+ open issues is just not acceptable. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary
This fixes the broken cross-references throughout the pyexpat documentation.
Test plan
🤖 Generated with Claude Code
📚 Documentation preview 📚: https://cpython-previews--144456.org.readthedocs.build/