Gordon P. Hemsley <gphems...@gphemsley.org> added the comment: >> As discussed above, starting with msg309074, __deepcopy__() is being added >> to the Python implementation because it already exists in the C >> implementation. > > Python implementation shouldn't copy all implementation details of the C > implementation. This is cumbersome and often impossible. Both implementation > should have the same behavior, and they do have.
As it stands, calling Element.__deepcopy__() will succeed with the C implementation and fail with the Python implementation. That seems problematic to me, and making that *not* be the case is trivial, so I don't understand the generalization to "all implementation details". Additionally, if copy.deepcopy() already works well with the Python implementation, why would it not work on the C implementation? What is different about the C implementation that requires the definition of __deepcopy__() in the first place? >> And additional tests have in fact uncovered further discrepancies between >> the Python and C implementations with regard to copying behavior. > > What are these discrepancies? As I mentioned in the PR, the C implementation does not make use of Element.__init__() when it creates new instances of the class, opting instead to use create_new_element() (which is not used in the C implementation of Element.__init__()). However, create_new_element() does not make a copy of the attrib dict that is passed in, meaning that the internal attrib dict of an Element instance is mutable by changing the dict that was passed in. I have modified the C implementation to fix this problem by adding a copy line in create_new_element(). Without this change, some of the new tests in the PR will fail in the C implementation. The Python implementation does not have this problem because it inherently always goes through Element.__init__(). > In any case it is better to extend existing tests by adding missed checks > than duplicate tests. As I mentioned in the PR: I have added the unit tests in the way that I have because the existing checks are generally integration tests, and most of the test coverage for this module comes as a side effect of pickle tests. It is my goal to expand the unit tests later to test each method individually, so starting that work here makes it easier. ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <https://bugs.python.org/issue32424> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
