| msg192239 - (view) |
Author: Olivier Gagnon (Olivier.Gagnon) |
Date: 2013-07-03 14:43 |
| The following code shows that the Counter is not deepcopied properly. The same code with an user defined class or a dict is copied with the "b" attribute. import collections import copy count = collections.Counter() count.b = 3 print(count.b) # prints 3 count_copy = copy.deepcopy(count) print(count_copy.b) # raise AttributeError: 'Counter' object has no attribute 'b' |
|
|
| msg192245 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) *  |
Date: 2013-07-03 15:58 |
| Here is a preliminary patch. It changes copying and pickling to preserve instance attributes. Actually I'm not sure which attributes should be copied. I doubt -- should this be considered as a fix or as a new feature? Perhaps OrderedDict should be changed in same way. |
|
|
| msg192248 - (view) |
Author: Eric Snow (eric.snow) * |
Date: 2013-07-03 17:51 |
| OrderedDict already copies the instance dict in __reduce__(). Are you talking about something more than that? |
|
|
| msg192256 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-07-03 20:49 |
| I mean OrderedDict.copy(). |
|
|
| msg192794 - (view) |
Author: Vajrasky Kok (vajrasky) * |
Date: 2013-07-10 10:57 |
| The question is whether we should give the freedom to user to add dynamic attribute to Counter object. Is this freedom has any practicality? Why do we want to add dynamic attributes to Counter object? Decimal object does not have this freedom. >>> from decimal import Decimal >>> z = Decimal('1.0') >>> z.foo = 'a' Traceback (most recent call last): File "", line 1, in AttributeError: 'decimal.Decimal' object has no attribute 'foo' Actually I am not really sure about this. Maybe someone knows the answer. |
|
|
| msg192813 - (view) |
Author: Olivier Gagnon (Olivier.Gagnon) |
Date: 2013-07-10 14:28 |
| The dictionary and the set do not give the freedom to add dynamic attributes to them. I agree that the Counter should have the same behaviour. However, this will raise the same bug when we inherit from a Counter object. >>> class mylist(list): pass ... >>> l = mylist() >>> l.foo = "bar" >>> c = copy.deepcopy(l) >>> print(c.foo) # prints bar >>> class myCounter(Counter): pass ... >>> original = myCounter() >>> original.foo = "bar" >>> c = copy.deepcopy(original) >>> c.foo Traceback (most recent call last): File "", line 1, in AttributeError: 'myCounter' object has no attribute 'foo' The reduction function should still copy every dynamic attribute of the object. |
|
|
| msg192872 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-07-11 14:30 |
| Here is an updated patch. It includes a fix for OrderedDict.copy(). Counter's copy() and __reduce__() are simplified. Added tests for OrderedDict which checks that OrderedDict copying and pickling preserves dynamic attributes. |
|
|
| msg192922 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2013-07-12 06:23 |
| I don't want the current behavior to change. The copy() method does not guarantee that it will copy added attributes. |
|
|
| msg192931 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-07-12 07:54 |
| What about pickling? OrderedDict.__reduce__() saves added attributes, but Counter.__reduce__() does not. One of them should be changed for consistency. |
|
|
| msg192936 - (view) |
Author: Olivier Gagnon (Olivier.Gagnon) |
Date: 2013-07-12 12:29 |
| I can understand that the current behaviour can be correct in regard with the added attributes of the object. However, should I open a new issue for the following inheritance behaviour which the reduce function affects also. class myCounter(Counter): def __init__(self, bar, *args): self.foo = bar super().__init__(*args) class myDict(dict): def __init__(self, bar, *args): self.foo = bar super().__init__(*args) c = myCounter("bar") l = myDict("bar") print(c.foo) # prints bar print(l.foo) # prints bar cc = copy.copy(c) ll = copy.copy(l) print(cc.foo) # prints {} print(ll.foo) # prints bar |
|
|
| msg192996 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2013-07-13 04:23 |
| Do you guys have any actual motivating use cases or real code that won't work because of the present design? Consistency arguments are somewhat weak and don't necessarily warrant an API change. AFAICT, there is no use for counters going out of their way to save attributes. Like many classes and types in Python, a subclasser is responsible for adding extra behavior if needed. |
|
|
| msg193010 - (view) |
Author: Olivier Gagnon (Olivier.Gagnon) |
Date: 2013-07-13 10:52 |
| Yes I do have code that break because of this behaviour. I'm doing evolutionary algorithm using a framework called DEAP. This framework creates a type called individual at the runtime by subclassing a container and adding it a fitness attribute. Those individual are copied as not to modify every indivual when we work on a single one. AFAIK the only container that can't be used right now is the counter because the fitness is not copied. I'm sure I can come up with a hack to have this behaviour, but it does clash with other standard container type and there is no mention anywhere that the Counter should be different than every other container type in the python standard library. |
|
|