[issue33274] minidom removeAttributeNode returns None
New submission from Ari Krupnik : W3C DOM Level 1[1] requires removeAttributeNode() to return the removed node: removeAttributeNode: Removes the specified attribute. Return Value: The Attr node that was removed. Minidom implementation returns None. [1]https://www.w3.org/TR/1998/REC-DOM-Level-1-19981001/level-one-core.html#method-removeAttributeNode -- components: Library (Lib), XML messages: 315253 nosy: iter priority: normal pull_requests: 6157 severity: normal status: open title: minidom removeAttributeNode returns None versions: Python 2.7, Python 3.8 ___ Python tracker <https://bugs.python.org/issue33274> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue33274] minidom removeAttributeNode returns None
Change by Ari Krupnik : -- type: -> behavior ___ Python tracker <https://bugs.python.org/issue33274> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue33274] minidom removeAttributeNode returns None
Ari Krupnik added the comment: I guess the main question if whether minidom wants to adhere to the standard or be pythonic, and it's not up to me to decide, although personally I like standards. The common use case for DOM functions returning the relevant nodes is for the caller to chain calls, e.g., e1.setAttributeNode(e0.removeAttributeNode(e0.getAttributeNode("a"))) instead of a=e0.getAttributeNode("foo") e0.removeAttributeNode(a) e1.setAttributeNode(a) -- ___ Python tracker <https://bugs.python.org/issue33274> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue33274] minidom removeAttributeNode returns None
Change by Ari Krupnik : -- keywords: +patch pull_requests: +7086 stage: test needed -> patch review ___ Python tracker <https://bugs.python.org/issue33274> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue33274] minidom removeAttributeNode returns None
Change by Ari Krupnik : -- pull_requests: +7087 ___ Python tracker <https://bugs.python.org/issue33274> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue33274] minidom removeAttributeNode returns None
Ari Krupnik added the comment: I added a test case and a News entry per serhiy.storchaka's request. https://github.com/python/cpython/pull/7465 I agree with fdrake's concerns about DOM's usefulness. DOM is not very Pythonic. I note that as long as Python has a DOM implementation, it follows the spec fairly closely. Element.removeAttributeNode() is the exception. All other minidom mutators (unpythonically) return the values that the standard requires, including the sister mutator setAttributeNode: Node.insertBefore() Node.appendChild() Node.replaceChild() Node.removeChild() NamedNodeMap.removeNamedItem() NamedNodeMap.removeNamedItemNS() NamedNodeMap.setNamedItem() Element.setAttributeNode() Document.removeChild() Document.renameNode() -- versions: +Python 2.7 ___ Python tracker <https://bugs.python.org/issue33274> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue33274] minidom removeAttributeNode returns None
Change by Ari Krupnik : -- pull_requests: -7086 ___ Python tracker <https://bugs.python.org/issue33274> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue33274] minidom removeAttributeNode returns None
Change by Ari Krupnik : -- pull_requests: +7088 ___ Python tracker <https://bugs.python.org/issue33274> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue33274] minidom removeAttributeNode returns None
Ari Krupnik added the comment: My bad. This issue looks like a simple omission to me--albeit one that's been in the code a long time. My patch simply brings the code into compliance with what the documentation (including in 2.7) already says it does. -- ___ Python tracker <https://bugs.python.org/issue33274> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue33274] minidom removeAttributeNode returns None
Ari Krupnik added the comment: I feel a little bit like I wandered into a card game whose rules I didn't understand. I'm just a lay, mortal user. I've been writing Python for 15 years, first time I saw an opportunity to contribute back. I saw what looked to me like a bug that's been in the code for 18 years, and I saw that it was a simple fix. You want a News entry? I'm happy to write a news entry. You want a full stop at the end? I'm happy to make that commit. You want me to write something in the doc? I'm happy to do that. What you do with my contribution is your call. I don't make the rules, like I said, I don't even understand them so well. When I first found this bug, I saw it as a very low-risk fix in terms of API change. It seems less likely that someone somewhere has code that depends on this function that always returning None. Python makes a very hard distinction between statements and expressions, and like you're saying, the Pythonic assumption is that a mutator is a statement whose return value one doesn't check. If this function returned a value, but the value were different from the spec, it would be a higher-risk change. As a side note, I'm a 2.7 user, so would benefit from backporting this fix. All this said, you're the maintainer, I'm the user, you don't have to justify your decisions to me. If you decide against backporting, I encourage you to update the documentation in earlier versions to state explicitly that this one mutator in the module diverges from the standard which the module otherwise implements faithfully. -- ___ Python tracker <https://bugs.python.org/issue33274> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue33274] minidom removeAttributeNode returns None
Ari Krupnik added the comment: In retrospect, I wish I had submitted this as a documentation change, simply acknowledging this function's behavior. "In most computer projects there comes a day when it is discovered that the machine and the manual don't agree. When the confrontation follows, the manual usually loses, for it can be changed far more quickly and cheaply than the machine." -- Fred Brooks, MMM, 1974 -- ___ Python tracker <https://bugs.python.org/issue33274> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com