Fix the problem with copy and deepcopy; also improve pickling a bit#311
Conversation
|
OK, it looks like Python 3.2 behaves the same way as Python 2, so that I added |
|
Also merged into CPython. |
| typing.Dict[T, Any], ClassVar[int], ClassVar[List[T]], Tuple['T', 'T'], | ||
| Union['T', int], List['T'], typing.Mapping['T', int]] | ||
| for t in things + [Any]: | ||
| self.assertEqual(t, copy(t)) |
There was a problem hiding this comment.
@ilevkivskyi: Are you sure this works correctly? From documentation:
It does “copy” functions and classes (shallow and deeply), by returning the original object unchanged; this is compatible with the way these are treated by the pickle module.
Test:
class Metaclass(type):
def __copy__(self):
return 1
class Foo(metaclass=Metaclass):
pass
>>> copy.copy(Foo)
<class '__main__.Foo'>
>>> Foo.__copy__()
1
>>> copy.copy(Foo) is Foo
TrueI do not think your test is really testing this correctly. Nor copying is really working correctly.
There was a problem hiding this comment.
Yes, we exactly follow the docs for copy module (classes are treated as immutable, except in Python 3.2 and 2.7, this is why we have __copy__ defined), what is the problem?
There was a problem hiding this comment.
Oh, so idea is that copy.copy should return same reference, and __copy__ is here just for compatibility with 3.2 and 2.7 to return something sane? But why is then doing self.__class__(...) and not just returning self? So why is really making a copy? Or you want in 3.2 and 2.7 to be a copy?
Should a test include that t is copy(t) on versions which are not 2.7 and 3.2?
There was a problem hiding this comment.
So why is really making a copy?
IIRC, to match the behaviour of 3.2 for other classes. Or maybe it is something to do with picke in 3.2, I am not sure now TBH.
Should a test include that
t is copy(t)on versions which are not 2.7 and 3.2?
This is not something super-important, but if you want to make a PR (with a comment quoting copy docs), then we will add it.
Fixes #306
@gvanrossum Here is the PR that I promised. I also improved pickling a bit, but as you could see from tests, the list of things that could be pickled is much shorter than list of those that can be copied/deepcopied (note that
copymodule in Python 3 treats classes as immutable, and therefore return the class unchanged)