Why deepcopy?

[Saluev]

I've been going through the code and stumbled upon deepcopy calls in frozendict's set, delete, etc. methods.

Why are they there? Isn't copy enough to add/remove a key?

Calling deepcopy for updates has obvious performance issues (it's slow to copy every object in the dict) so in my project I removed these calls via overriding the corresponding methods. But maybe I'm missing something?

[Marco-Sulla]

Why are they there? Isn't copy enough to add/remove a key?

Of course not:

>>> from frozendict import frozendict as fd
>>> a = fd({1:[]})
>>> b = a.copy()
>>> a[1].append(1)
>>> b
frozendict.frozendict({1: [1]})
>>>

The goal of methods like set is to have a functional style of programming.

Anyway, deepcopy is in the Python code. The C extension is faster. See the benchmarks.

[Saluev]

Ahh right. Didn't think of this because I kinda thought frozendicts' primary purpose was to be hashable. Thanks for clarifying!

I get the idea now, but I have to note that this behaviour contradicts behaviour of built-in Python immutables like tuple:

a = ([],)
b = a + ([],)
a[0].append(1)
b

I believe they don't care about nested immutability precisely because of performance considerations.

[Marco-Sulla]

I kinda thought frozendicts' primary purpose was to be hashable

Good point. In Python code I can avoid the deepcopy if the frozendict is hashable.

this behaviour contradicts behaviour of built-in Python immutables like tuple

Not at all. Your code is similar to the example I posted above. The real difference between frozendict and tuple is that tuple has not a set method, because CPython devs are not interested in officially supporting functional programming. On the contrary, frozendict will be part of the stdlib, since it's quite easy to do

[Saluev]

The real difference between frozendict and tuple is that tuple has not a set method

Well, tuple has __add__ which sounds essentially the same to me. And frozen dataclasses have replace which is more similar and also doesn't do deepcopy. Anyway, looking forward to seeing frozendict in the standard library!

[Marco-Sulla]

Ok, there's nothing to optimize. __deepcopy__ already returns copy if frozendict is hashable:

https://github.com/Marco-Sulla/python-frozendict/blob/master/src/frozendict/_frozendict_py.py#L132-L148

[Marco-Sulla]

Mh, anyway to make this effective I have to do in set etc:

dict(deepcopy(self))

instead of

deepcopy(dict(self))

[Marco-Sulla]

Released frozendict 2.4.5. Ty for the tip.

[Marco-Sulla]

Well, tuple has __add__ which sounds essentially the same to me. And frozen dataclasses have replace which is more similar and also doesn't do deepcopy.

My apologize. You are very right. Furthermore, I checked the behavior of the C extension, and it does not do a deepcopy. So there's an inconsistency between the C and the py implementations. I'll fix it removing the deepcopy call in the py version.

[Marco-Sulla]

Released frozendict 2.4.6. Can you confirm the issue is fixed now, @Saluev?