pysfml-cython icon indicating copy to clipboard operation
pysfml-cython copied to clipboard

deepcopy

Open sanchopanca opened this issue 12 years ago • 8 comments

Python 2.7.1+ (r271:86832, Apr 11 2011, 18:05:24) [GCC 4.5.2] on linux2 Type "help", "copyright", "credits" or "license" for more information.

from PySFML import sf c = sf.Color(123, 123, 123) l = [[c]] import copy l2 = copy.deepcopy(l) l2 [[]] l2[0][0].r 0L l2[0][0].b 0L l2[0][0].g 0L

Documentation: In order for a class to define its own copy implementation, it can define special methods copy() and deepcopy().

sanchopanca avatar Mar 10 '12 22:03 sanchopanca

I reopen this, so that I don't forget it in the future.

By the way, is seems very dangerous to support deepcopy: it means that the user could easily duplicates heavy objects like images without noticing it. I don't see any reason why you would want to duplicate these objets, so I don't think I will support deepcopy, unless maybe for more simple classes. Color only contains ints, so copy and deepcopy don't make any difference, right?

bastienleonard avatar Mar 11 '12 09:03 bastienleonard

Sorry for my English, I'm from Russia. I think that a user who uses deepcopy understands that there is a complete copy of all embedded objects.

sanchopanca avatar Mar 11 '12 14:03 sanchopanca

As I said, I can't think of one good for duplicating heavy objects like images. That's already a good reason for not supporting it, in my opinion. The fact that novice users could easily confuse copy and deepcopy is another reason.

bastienleonard avatar Mar 16 '12 19:03 bastienleonard

I think it's better to target the final user than the novice. If a beginner makes this mistake, he'll be warned by someone of the community, or learn it by himself. In addition, you can just put it in the doc/tutorials .

alexandre-janniaux avatar Mar 17 '12 20:03 alexandre-janniaux

I have added copy() methods in some classes: Sprite, Time, Transform, FloatRect and IntRect (Vector2f already had one). So far, I don't see how deep copy support would be useful for these classes, so I'm not using the copy module features yet. Feel free to discuss.

bastienleonard avatar May 22 '12 15:05 bastienleonard

I don't think most novices even know there is a deepcopy function in the copy module (or that there's a copy module for that matter), so I think your fears are a non-issue.

That said, if you were really paranoid, you could be extra annoying and add a module-level variable called ALLOW_DEEP_COPY which would be set to False by default. If not enabled, a warning saying that copy is preferable as deepcopy is an expensive operation.

>>> from copy import deepcopy
....
>>> image_copy = deepcopy(image)
Exception: Deep copying is an expensive operation, and has been disabled. If you really need this functionality, set 
sfml.ALLOW_DEEP_COPY to True
>>> sfml.ALLOW_DEEP_COPY = True
>>> image_copy = deepcopy(image)
>>>

Personally, I think this is overkill, but if previous arguments are convincing enough, I think this serves as a decent compromise. Failing that, I suppose the others could simply subclass Sprite (or whatever) and implement deepcopy themselves...

AphonicChaos avatar May 23 '12 00:05 AphonicChaos

What would an image deep copy provide that a normal doesn't? Image doesn't hold a reference to any Python object, so it should be the same. Sprite has a reference to the texture, but as far as I know, having the same texture loaded several times in memory would always be a bug. So I don't want to provide deep copy in this case. As far as I can tell, the other classes that currently have a copy() method don't hold any reference to mutable Python objects.

bastienleonard avatar May 23 '12 09:05 bastienleonard

I don't know. Personally, I can live without deep copy. I was throwing it out there as a suggestion, considering the issue is still open.

AphonicChaos avatar May 23 '12 10:05 AphonicChaos