Repository navigation
Synchronize copy methods between Python and C implementations of xml.etree.ElementTree.Element #76605
Description
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2019-09-10.15:22:43.606>
created_at = <Date 2017-12-24.18:51:40.307>
labels = ['expert-XML', '3.8', 'type-bug', 'library', '3.9']
title = 'Synchronize copy methods between Python and C implementations of xml.etree.ElementTree.Element'
updated_at = <Date 2019-09-10.15:22:43.606>
user = 'https://github.com/GPHemsley'bugs.python.org fields:
activity = <Date 2019-09-10.15:22:43.606>
actor = 'scoder'
assignee = 'none'
closed = True
closed_date = <Date 2019-09-10.15:22:43.606>
closer = 'scoder'
components = ['Library (Lib)', 'XML']
creation = <Date 2017-12-24.18:51:40.307>
creator = 'gphemsley'
dependencies = []
files = []
hgrepos = []
issue_num = 32424
keywords = ['patch']
message_count = 30.0
messages = ['309010', '309074', '309089', '309116', '309121', '309122', '309124', '309131', '309140', '309142', '309143', '309144', '309145', '309146', '309147', '309149', '309179', '309184', '309263', '309953', '310362', '340381', '340589', '340592', '340600', '340602', '341021', '341447', '351698', '351699']
nosy_count = 9.0
nosy_names = ['scoder', 'r.david.murray', 'eli.bendersky', 'asvetlov', 'serhiy.storchaka', 'thatiparthy', 'mdk', 'p-ganssle', 'gphemsley']
pr_nums = ['5022', '5046', '12891', '12995']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue32424'
versions = ['Python 3.8', 'Python 3.9']
Activity
GPHemsley commentedon Dec 24, 2017
Currently, the Python implementation of the Element class in xml.etree.ElementTree defines a method called copy() which the C implementation does not define, whereas the C implementation defines a __copy__() method (and a __deepcopy__() method) which the Python implementation does not define.
Given that the documentation makes no mention of a copy() method and that its definition would be masked by a standard import of xml.etree.ElementTree, I propose that it be renamed to __copy__() so that copy.copy() can make use of it.
JulienPalard commentedon Dec 26, 2017
As the C implementation is already shadowing the Python implementation when available (last lines of Lib/xml/etree/ElementTree.py), using
.copy()is already a bug magnet (works when the Python implem is used, does not work when shadowed by the C implem).So renaming the copy method to __copy__ in the python implementation looks good to me.
Ultimately the Python implementation would require a __deepcopy__ too?
GPHemsley commentedon Dec 27, 2017
Ultimately, yeah, the Python version should probably define __deepcopy__ as well. But since this is just a rename of an existing method, I figure we can defer that to another time.
srinivasreddy commentedon Dec 28, 2017
Renaming the method
copy()to__copy__breaks the API. I would rather have an alias for__copy__.I agree that it is not documented, but some users tend to assume public methods are documented and use them.
So i think it is better to not to break their code.
asvetlov commentedon Dec 28, 2017
Agree with Srinivas
JulienPalard commentedon Dec 28, 2017
I don't agree with the API breakage, the C implementation of the module does not expose copy(), so code using copy() is already broken (since 2005) in cases the C implementation is used.
I also expect the C implementation to be used almost anywhere, but I may be wrong on this point: in which case the Python implementation may still be used?
asvetlov commentedon Dec 28, 2017
pypy for example has Element.copy()
GPHemsley commentedon Dec 28, 2017
Two notes:
That said, I'm neutral on the subject. I'm happy to implement whichever option you decide on.
asvetlov commentedon Dec 28, 2017
I mean dropping
.copy()breaks not only compatibility between pure python versions but also between pypy releases and pypy/CPython code.That's why I suggest to keep
.copy()as an alias for__copy__().pganssle commentedon Dec 28, 2017
If the main issue is that pypy users have been using this, can't pypy just keep an alias for
copyaround, possibly with a DeprecationWarning?asvetlov commentedon Dec 28, 2017
For derivative Python implementation there is a standard strategy: they uses a copy of CPython standard library.
Tthat's why any new module should have a pure Python implementation and that's why https://www.python.org/dev/peps/pep-0399/ exists.
Jython has copy() method as well. IronPython has it. I doubt if an implementation without xml.etree.ElementTree.Element.copy exists at all.
The method removal breaks backward compatibility without a reason, __copy__ alias is valuable and backward compatible change.
11 remaining items
serhiy-storchaka commentedon Apr 17, 2019
You should not call it directly. Use copy.deepcopy().
__deepcopy__() in the C implementation is merely optimization. It is less efficient to convert from the efficient internal representation of Element object to tuples and dicts and back.
create_new_element() usually is called with a new copy of the attrib dict. Making yet one copy inside it is just a waste of time. If in some cases it is called with an externally referred dict, fix these cases by making a copy before calling create_new_element().
GPHemsley commentedon Apr 21, 2019
Taking a step back, I want to make sure I understand the action items for resolving this:
Any other issues discussed here are orthogonal to the goal of this ticket.
Are we in agreement on this?
GPHemsley commentedon Apr 21, 2019
Opened bpo-36685 for discussion of the attrib copy issue.
scoder commentedon Apr 21, 2019
We should not add anything to the implementation that we consider legacy elsewhere. Py3 has "always" used the C accelerator module instead of the Python implementation, which suggests that its interface is probably the righter one.
So: make sure that the Obvious Ways, copy.copy() and copy.deepcopy(), work nicely for both implementations, and deprecate Element.copy() in Py3.8.
serhiy-storchaka commentedon Apr 21, 2019
Concur with Stefan. Adding the method deprecated from the born looks silly. We should either deprecate the copy() method in the Python implementation, or add an undeprecated method in the C implementation. The latter should be considered as a new feature.
scoder commentedon Apr 28, 2019
New changeset 50fed0b by Stefan Behnel (Gordon P. Hemsley) in branch 'master':
bpo-32424: Improve test coverage for xml.etree.ElementTree (GH-12891)
50fed0b
GPHemsley commentedon May 5, 2019
It seems the final open question on this is whether the mismatch between the Python and C implementations is enough to bypass PendingDeprecationWarning for copy() in favor of jumping straight to DeprecationWarning.
scoder commentedon Sep 10, 2019
Let's just deprecate it directly (in Py3.9, I guess), given that it was never documented.
scoder commentedon Sep 10, 2019
New changeset 7d952de by Stefan Behnel (Gordon P. Hemsley) in branch 'master':
bpo-32424: Deprecate xml.etree.ElementTree.Element.copy() in favor of copy.copy() (GH-12995)
7d952de
vstinner commentedon Jun 28, 2022
Follow-up: issue #94383: Remove xml.etree.ElementTree.Element.copy() (Python implementation): use copy.copy() instead.