Fix: Enable freezing collections.defaultdict objects #281
Conversation
pyrsistent/_helpers.py
Outdated
| @@ -31,7 +31,7 @@ def freeze(o, strict=True): | |||
| (1, pvector([])) | |||
| """ | |||
| typ = type(o) | |||
| if typ is dict or (strict and isinstance(o, PMap)): | |||
| if isinstance(o, dict) or (strict and isinstance(o, PMap)): | |||
There was a problem hiding this comment.
While this certainly works for what you want to achieve I'm sligthly worried of the unintended effects it may have in clients code bases. Things that may not previously have been froozen (such as OrderedDicts , Counters, etc.) now start becoming frozen which leads to data loss and change of existing functionality.
If we want freeze to work also for defaultdicts I would prefer adding it as a distinct type to check against (using is) given the above considerations. Although not as nice/general I think it would be a safer move with a smaller blast radius. It's still a backwards imcompatible change, but a smaller one.
Also, please add a test for your new case while you are at it. :-)
There was a problem hiding this comment.
Makes sense! Thank you for the review and suggestions.
I have made the changes accordingly.
Also, I've added tests for defaultdict. :-)
…shad-ml/pyrsistent into fix/handle-freeze-defaultdict
|
Thanks! |
Fixes #280
Solution:
Changing the condition from
typ is dicttoisinstance(o, dict)enables thefreezefunction to convert objects of collections.defaultdict type toPmap.