Behavior of immutable attributes

[CleitonDeLima]

When using get_inlines and get_inlines_names, a new copy of the object is returned, avoiding changing the class attribute.

[codecov-commenter]

Codecov Report

Merging #237 (7d11b65) into master (415ef28) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #237      +/-   ##
==========================================
+ Coverage   88.77%   88.79%   +0.02%     
==========================================
  Files           6        6              
  Lines         490      491       +1     
  Branches       54       54              
==========================================
+ Hits          435      436       +1     
  Misses         40       40              
  Partials       15       15              

Impacted Files	Coverage Δ
extra_views/advanced.py	96.33% <100.00%> (+0.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 415ef28...7d11b65. Read the comment docs.

[sdolemelipone]

Thanks, looks like a good idea. @jonashaag are you good to merge?

[jonashaag]

Is there something similar or identical in Django? Is copying a common approach in the Django codebase?

[sdolemelipone]

When mutable class level attributes are returned by instance methods they are generally copied, e.g. see FormMixin.get_initial() here: https://github.com/django/django/blob/main/django/views/generic/edit.py

This commit differs from Django in that it is using deepcopy() rather than a shallow .copy() or [:].

[jonashaag]

Thank you! Shouldn’t we use copy as well? We don’t want to make copies of the classes, only return a fresh dict.

[sdolemelipone]

Yes, I think you're right! I like how asking the right question gets the right answer :-)

@CleitonDeLima would you re-write this to use [:] instead of deepcopy? Or can you suggest why deepcopy would be preferred over a shallow copy?

[CleitonDeLima]

Yes, I think you're right! I like how asking the right question gets the right answer :-)

@CleitonDeLima would you re-write this to use [:] instead of deepcopy? Or can you suggest why deepcopy would be preferred over a shallow copy?

Hi @sdolemelipone ,
It really could just be the copy, I can change that.
I didn't use [:] because tuples can be used.

a = (1, 2, 3)
id(a) == id(a[:])  # is True

[sdolemelipone]

Thanks! LGTM.

Just as a side discussion, as tuples are immutable, does the [:] matter? I think we are only protecting against mutable lists being modified via the instance.

[CleitonDeLima]

Thanks! LGTM.

Just as a side discussion, as tuples are immutable, does the [:] matter? I think we are only protecting against mutable lists being modified via the instance.

Of course! I'm getting in my way on basic concepts, treat the problem with [:] is enough. 😅
Thanks!

[sdolemelipone]

Even better! 😄 Thanks, I hadn't thought about what happens when you call [:] on a tuple until now.

[jonashaag]

I'd have preferred the explicit copy way personally but this is fine. Thanks everyone!

[sdolemelipone]

I'd have preferred the explicit copy way personally but this is fine. Thanks everyone!

Interesting! Why, because explicit is better than implicit? For me [:] appeared more beautiful/elegant.
Also, I didn't know that:

from copy import copy
a = (1, 2, 3)
id(a) == id(copy(a))  # is True

[jonashaag]

Yes, and because it’s done that way in the Django codebase. Needlessly doing things differently in one place than in the other increases chances of creating bugs, adds to the mental load of readers, etc.