gh-128508: Add some docstrings to xml.dom.minidom by srinivasreddy · Pull Request #128477 · python/cpython · GitHub

Initial version extracted from PR #24152, which also deleted and modified minidom tests.

Co-authored-by: Karl Dubost karl+github@la-grange.net

Issue: Add some docstrings to xml.dom.minidom #128508

I have not worked on docstrings, especially not for anything other that IDLE, for years. The best reference I know is the Multiline String section of https://peps.python.org/pep-0257/, though the thoroughness described there may be rare. Still, these need more work. The xml.dom and xml.dom.minidom docs should be a basis for corresponding docstrings. Usefully summarizing behavior can be hard.

I think a blurb should be added, but this can wait.

Remove \n after """. Add after this line. But this revision seems wrong as it omits mention of newOwnerDocument. I would have to read code to suggest improvement.

Should be "Return a duplicate ...", but needs to explain 'deep' (I suspect it is a boolean.) Docstrings are for quick reminders when coding, and 'deep' is not obvious.

See other cloneNode comment.

Suggested change

      
                    """Replace an existing node with a new node."""
          
                    """Replace child node *oldChild* with *newChild*."""

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.

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

…ide C comments (python#128464)

…o()` (python#128337) These methods combine `_delete()` and `copy()`, but `_delete()` isn't part of the public interface, and it's unlikely to be added until the pathlib ABCs are made official, or perhaps even later.

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>

…er test (python#128338) In the tests for `pathlib.Path.walk()`, avoid using the path class under test (`self.cls`) in test setup. Instead we use `os` functions in `test_pathlib`, and direct manipulation of `DummyPath` internal data in `test_pathlib_abc`.

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

…127564)

…on#128502)

pythongh-128455) Add `BOLT_COMMON_FLAGS` with `-update-debug-sections` Co-authored-by: Gregory Szorc <gregory.szorc@gmail.com>

…he atomic operation (pythongh-128196)

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

I think it’s strange to have a class docstring start with a verb; it makes sense for a method (that does something), but a class usually represents a thing (noun), it does not do something.

Addressed in b97d812

I think the docstring should document what the method does, not quote parts of a spec.

I would suggest simpler wording, but I don’t understand the existing one («the result of pre-inserting node into this before child» ?!)

Addressed in b97d812

I would have links like this in the reST docs, not docstrings.

Addressed in b97d812

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

This PR is stale because it has been open for 30 days with no activity.

The formatting concerns from the earlier reviews look addressed. Some remarks on the content:

toxml() returns bytes, not a string, if encoding is given. toprettyxml(), which does the actual work and has the interesting parameters, has no docstring.
"exclusive Text nodes" in normalize() is not DOM terminology. It is also worth mentioning that the method is recursive.
DocumentType.cloneNode() returns None if the node has an owner document, so the docstring copied from Node.cloneNode() does not describe it.
The change to _clone_node() is unrelated: it is private and already had a docstring, and the note "Called by Node.cloneNode and Document.importNode" was more useful than the parameter list replacing it.

Also, this only adds a docstring for the Node class; Document, Element, Attr and others still have none, which is what gh-57952 was left open for.

Update and Add docstrings for functions and methods in minidom.py module

21bb374

bedevere-app Bot added the awaiting review label Jan 4, 2025

srinivasreddy commented Jan 4, 2025

View reviewed changes

Comment thread Lib/xml/dom/minidom.py Outdated

srinivasreddy changed the title Update and Add docstrings for functions and methods in minidom.py module gh-63882: Update and Add docstrings for functions and methods in minidom.py module Jan 4, 2025

bedevere-app Bot mentioned this pull request Jan 4, 2025

test_minidom has many empty tests #63882

Closed

terryjreedy changed the title gh-63882: Update and Add docstrings for functions and methods in minidom.py module gh-128508: Update and Add docstrings for functions and methods in minidom.py module Jan 5, 2025

bedevere-app Bot mentioned this pull request Jan 5, 2025

Add some docstrings to xml.dom.minidom #128508

Open

terryjreedy added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Jan 5, 2025

terryjreedy changed the title gh-128508: Update and Add docstrings for functions and methods in minidom.py module gh-128508: Add some docstrings to xml.dom.minidom Jan 5, 2025

terryjreedy requested changes Jan 5, 2025

View reviewed changes

bedevere-app Bot removed the awaiting review label Jan 5, 2025

bedevere-app Bot added the awaiting changes label Jan 5, 2025

terryjreedy mentioned this pull request Jan 5, 2025

gh-63882: Adds tests for xml.dom.minidom #24152

Closed

srinivasreddy and others added 15 commits January 6, 2025 11:34

Update Lib/xml/dom/minidom.py

f16761b

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

Update Lib/xml/dom/minidom.py

eb0208b

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

Update Lib/xml/dom/minidom.py

f4f6334

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

Add check-readthedocs pre-commit hook (python#128453 )

70617ea

pythongh-128152 : Argument Clinic: ignore pre-processor directives ins…

90b344f

…ide C comments (python#128464)

pythonGH-127381 : pathlib ABCs: remove PathBase.move() and `move_int…

34c317a

…o()` (python#128337) These methods combine `_delete()` and `copy()`, but `_delete()` isn't part of the public interface, and it's unlikely to be added until the pathlib ABCs are made official, or perhaps even later.

Docs: mark up json.dump() using parameter list (python#128482 )

5f7ba82

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>

pythongh-126719 : Clarify math.fmod docs (python#127741 )

e6a8d38

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Docs: amend json.dump() post pythongh-128482 (python#128489 )

1592145

pythongh-127954 : Document PyObject_DelItemString (python#127986 )

1a8ee69

pythongh-127553 : Remove outdated TODO comment in _pydatetime (python#…

61c3e8a

…127564)

pythongh-115765 : Document and enforce Autoconf 2.72 requirement (pyth…

69f8e4b

…on#128502)

pythongh-128437 : Add BOLT_COMMON_FLAGS with -update-debug-sections (

7f767cd

pythongh-128455) Add `BOLT_COMMON_FLAGS` with `-update-debug-sections` Co-authored-by: Gregory Szorc <gregory.szorc@gmail.com>

pythongh-128137 : Update PyASCIIObject to handle interned field with t…

2f8b072

…he atomic operation (pythongh-128196)

erlend-aasland reviewed Jan 6, 2025

View reviewed changes

Comment thread Lib/xml/dom/minidom.py

srinivasreddy added 6 commits January 6, 2025 15:06

Update docstring as recommended by argument clinic

38be045

Update docstring as recommended by argument clinic

cca0fda

Undo removing comment

8df6795

Update docstrings as it was done in argument clinic

727af86

Add space back

2da171f

Update doc strings

ecb4a54

erlend-aasland reviewed Jan 6, 2025

View reviewed changes

Comment thread Lib/xml/dom/minidom.py Outdated

Comment thread Lib/xml/dom/minidom.py Outdated

srinivasreddy and others added 2 commits January 6, 2025 18:13

Update Lib/xml/dom/minidom.py

8b63730

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

Update Lib/xml/dom/minidom.py

49a12e6

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

srinivasreddy requested a review from erlend-aasland January 7, 2025 09:45

merwok reviewed Jan 9, 2025

View reviewed changes

srinivasreddy added 2 commits January 16, 2025 18:17

Merge branch 'main' into pythongh-63882 -doc_strings

298a20f

Address review comments

b97d812

srinivasreddy requested a review from merwok January 16, 2025 13:03

Update docstrings

19d245c

srinivasreddy mentioned this pull request Jan 17, 2025

gh-63882: Comment out empty test_minidom tests #128948

Closed

tomasr8 removed the needs backport to 3.12 only security fixes label Apr 10, 2025

serhiy-storchaka added the needs backport to 3.14 bugs and security fixes label May 8, 2025

erlend-aasland reviewed May 12, 2025

View reviewed changes

Comment thread Lib/xml/dom/minidom.py

srinivasreddy and others added 2 commits May 13, 2025 09:45

Update Lib/xml/dom/minidom.py

7b1666d

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

Merge branch 'main' into pythongh-63882 -doc_strings

c68cb4d

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 20, 2026

serhiy-storchaka added the needs backport to 3.15 bugs and security fixes label May 30, 2026

github-actions Bot removed the stale Stale PR or inactive for long period of time. label May 31, 2026

serhiy-storchaka mentioned this pull request Aug 30, 2026

xml.dom.minidom.Document class is not documented #57952

Closed

+                      https://dom.spec.whatwg.org/#dom-node-insertbefore
+                      The insertBefore(node, child) method, when invoked,
+                      must return the result of pre-inserting node into
+                      this before child.

+                      The insertBefore(node, child) method, when invoked,
+                      must return the result of pre-inserting node into
+                      this before child.
+                      See also https://dom.spec.whatwg.org/#concept-node-pre-insert

                   """
-                  Clone a node and give it the new owner document.
+                  Returns a copy of node.

                       self.childNodes[:] = L
                   def cloneNode(self, deep):
+                      """The Node.cloneNode() method returns a duplicate of the node."""

                       return node
                   def replaceChild(self, newChild, oldChild):
+                      """Replace an existing node with a new node."""

	"""Replace an existing node with a new node."""
	"""Replace child node oldChild with newChild."""



		class Node(xml.dom.Node):
		"""Define properties accessible on a DOM node."""

gh-128508: Add some docstrings to xml.dom.minidom by srinivasreddy · Pull Request #128477 · python/cpython · GitHub

Repository navigation

gh-128508: Add some docstrings to xml.dom.minidom - #128477

gh-128508: Add some docstrings to xml.dom.minidom#128477srinivasreddy wants to merge 38 commits intopython:mainpython/cpython:mainfrom srinivasreddy:gh-63882-doc_stringssrinivasreddy/cpython:gh-63882-doc_stringsCopy head branch name to clipboard

Conversation

srinivasreddy commented Jan 4, 2025 • edited by terryjreedy Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

terryjreedy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-app Bot commented Jan 5, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Apr 20, 2026

Uh oh!

serhiy-storchaka commented Aug 30, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants

gh-128508: Add some docstrings to xml.dom.minidom#128477
srinivasreddy wants to merge 38 commits into
python:mainfrom
srinivasreddy:gh-63882-doc_strings

srinivasreddy commented Jan 4, 2025 •
edited by terryjreedy
Loading

terryjreedy left a comment •
edited
Loading