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 — Add class directives for ``xml.dom`` by AA-Turner · Pull Request #138419 · python/cpython

Conversation

@AA-Turner

@AA-Turner AA-Turner commented Sep 3, 2025 •
edited by github-actions Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Required for #138418.

A


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

Sorry, something went wrong.

Uh oh!

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

All reactions
@AA-Turner
Add class directives for xml.dom
ea6a659
@AA-Turner AA-Turner added skip issue needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes labels Sep 3, 2025
@bedevere-app bedevere-app Bot added awaiting core review docs Documentation in the Doc dir skip news labels Sep 3, 2025
@github-project-automation github-project-automation Bot added this to Docs PRs Sep 3, 2025
@github-project-automation github-project-automation Bot moved this to Todo in Docs PRs Sep 3, 2025
@AA-Turner AA-Turner mentioned this pull request Sep 3, 2025
gh-101100: Resolve reference warnings in reference/ #138418
Merged
serhiy-storchaka
serhiy-storchaka reviewed Sep 3, 2025
View reviewed changes
Comment thread Doc/library/xml.dom.rst Resolved

Uh oh!

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

Comment thread Doc/library/xml.dom.rst Resolved

Uh oh!

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

AA-Turner added 2 commits September 3, 2025 16:27
@AA-Turner
Suppress self-references
7f8a079
@AA-Turner
Serhiy's review (getElementsByTagNameNS not defind in Node)
36d6f41
@AA-Turner
AA-Turner requested a review from serhiy-storchaka September 3, 2025 15:29
serhiy-storchaka
serhiy-storchaka reviewed Sep 3, 2025
View reviewed changes

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

I have great doubts about these changes. Because all these classes are not defined in the xml.dom module. This is not description of concrete classes, this is description of ephemeral interfaces. Concrete classes are defined in the minidom submodule and in third-party modules. Adding class directives here will create wrong index entries and add wrong user visible information.

Try to add :no-typesetting: and :no-index-entry: options. Not sure about :no-contents-entry:. Will this be of any benefit?

Sorry, something went wrong.

Uh oh!

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

All reactions
@AA-Turner

AA-Turner commented Sep 3, 2025

Copy link
Copy Markdown
Member Author

This is not description of concrete classes, this is description of ephemeral interfaces.

Sphinx doesn't have a .. py:interface:: or .. py:protocol:: directive, so class is the best thing we can do. Would you be happier with an admonition on each directive reminding the reader that they're protocol classes?

I think it would be useful to add them, but I'm happy to revert to just adding the targets if you still have reservations.

A

All reactions

Sorry, something went wrong.

Uh oh!

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

@serhiy-storchaka

serhiy-storchaka commented Sep 3, 2025

Copy link
Copy Markdown
Member

User should not see class Element here, nor the entry "Element (xml.dom class)", because xml.dom.Element does not exist.

It would be nice to make some class references referring to corresponding section in this file, for example, :class:`Document` in the xml.dom.minidom.parse() description to Document Objects. But not for all (for example NodeList does not exist, it is just a list). Sphinx is bad in using nested markup, so we cannot make a link from Document formatted as class to arbitrary anchor. Can we? If this is possible, it would be an ideal variant -- make all class references inert and add links manually.

Before we find a satisfying solution, it is better to leave unresolved references as a reminder.

All reactions

Sorry, something went wrong.

Uh oh!

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

@AA-Turner

AA-Turner commented Sep 3, 2025

Copy link
Copy Markdown
Member Author

Ok, I will close this for now & we can revisit it later, I will make a smaller change to #138418.

A

All reactions

Sorry, something went wrong.

Uh oh!

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

@AA-Turner AA-Turner closed this Sep 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments

Uh oh!

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

Assignees

No one assigned

Labels

awaiting core review docs Documentation in the Doc dir needs backport to 3.13 only security fixes needs backport to 3.14 bugs and security fixes skip issue skip news

Projects

Docs PRs
Status: Todo

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.

2 participants

@AA-Turner @serhiy-storchaka
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.