Python function that deeply "freezes" an object

Question

I implemented a function that gets an object and returns its immutable counterpart. If the object contains another mutable object at any level, it freezes it too.

Is my code is good and/or can it be improved?

https://github.com/Marco-Sulla/python-frozendict/blob/master/src/frozendict/cool.py#L163-L309

def deepfreeze(
 o,
 custom_converters = None,
 custom_inverse_converters = None
):
 r"""
 Converts the object and all the objects nested in it in its
 immutable counterparts.
 
 The conversion map is in getFreezeConversionMap().
 
 You can register a new conversion using `register()` You can also
 pass a map of custom converters with `custom_converters` and a map
 of custom inverse converters with `custom_inverse_converters`,
 without using `register()`.
 
 By default, if the type is not registered and has a `__dict__`
 attribute, it's converted to the `frozendict` of that `__dict__`.
 
 This function assumes that hashable == immutable (that is not
 always true).
 
 This function uses recursion, with all the limits of recursions in
 Python.
 
 Where is a good old tail call when you need it?
 """
 
 from frozendict import frozendict
 
 if custom_converters is None:
 custom_converters = frozendict()
 
 if custom_inverse_converters is None:
 custom_inverse_converters = frozendict()
 
 for type_i, converter in custom_converters.items():
 if not issubclass(type(type_i), type):
 raise ValueError(
 f"{type_i} in `custom_converters` parameter is not a " +
 "type"
 )
 
 try:
 converter.__call__
 except AttributeError:
 raise ValueError(
 f"converter for {type_i} in `custom_converters` " + 
 "parameter is not a callable"
 )
 
 for type_i, converter in custom_inverse_converters.items():
 if not issubclass(type(type_i), type):
 raise ValueError(
 f"{type_i} in `custom_inverse_converters` parameter " +
 "is not a type"
 )
 
 try:
 converter.__call__
 except AttributeError:
 raise ValueError(
 f"converter for {type_i} in " +
 "`custom_inverse_converters`parameter is not a callable"
 )
 
 type_o = type(o)
 
 freeze_types = tuple(custom_converters.keys()) + getFreezeTypes()
 
 base_type_o = None
 
 for freeze_type in freeze_types:
 if isinstance(o, freeze_type):
 base_type_o = freeze_type
 break
 
 if base_type_o is None:
 # this is before hash check because all object in Python are
 # hashable by default, if not explicitly suppressed
 try:
 o.__dict__
 except AttributeError:
 pass
 else:
 return frozendict(o.__dict__)
 
 try:
 hash(o)
 except TypeError:
 pass
 else:
 # without a converter, we can only hope that
 # hashable == immutable
 return o
 
 supported_types = ", ".join((x.__name__ for x in freeze_types))
 
 err = (
 f"type {type_o} is not hashable or is not equal or a " +
 f"subclass of the supported types: {supported_types}"
 )
 
 raise TypeError(err)
 
 freeze_conversion_map = getFreezeConversionMap()
 
 freeze_conversion_map = freeze_conversion_map | custom_converters
 
 if base_type_o in _freeze_types_plain:
 return freeze_conversion_map[base_type_o](o)
 
 if not isIterableNotString(o):
 return freeze_conversion_map[base_type_o](o)
 
 freeze_conversion_inverse_map = getFreezeConversionInverseMap()
 
 freeze_conversion_inverse_map = (
 freeze_conversion_inverse_map |
 custom_inverse_converters
 )
 
 frozen_type = base_type_o in freeze_conversion_inverse_map
 
 if frozen_type:
 o = freeze_conversion_inverse_map[base_type_o](o)
 
 from copy import copy
 
 o_copy = copy(o)
 
 for k, v in getItems(o_copy)(o_copy):
 o_copy[k] = deepfreeze(
 v,
 custom_converters = custom_converters,
 custom_inverse_converters = custom_inverse_converters
 )
 
 try:
 freeze = freeze_conversion_map[base_type_o]
 except KeyError:
 if frozen_type:
 freeze = type_o
 else: # pragma: no cover
 raise
 
 return freeze(o_copy)

Please feel free to ask everything about the code. I can also improve the docs and comments if you feel it's needed.

• Mast: What's the use case for your code?
• Marco Sulla: @Mast Well, it transforms an object in an immutable object, that can be used in sets and dicts, or simply because you want to be sure the object will be not modified (even if you can always assign to the variable holding the immutable object a mutable one. It's Python anyway, so this is the best I can do :) )

Answer 1

Thank you for the GitHub url for context; that's very helpful. I am also reading about the project on pypi.

I find the motivating Use Case a bit obscure, and had trouble finding it in the very nice automated test suite. As an app developer, why would I want to call into this library? It feels like there may be two things going on here:

1. Adversarial code wants to mutate an attribute, even though I said "freeze!".
2. We might desire a 1:1 correspondence between a frozen object and its serialized representation on disk, perhaps in a *.json file.

I didn't notice examples of either of those in the repo, though I suppose I might have looked a bit harder.

I get the sense that a promise of "I will freeze anything!" is perhaps too strong. But I don't know what rules an app developer should obey. Sticking to "JSON serializability" seems like a pretty safe, conservative guideline to offer.

imports at top

def deepfreeze(
 o,
 ...
 
 from frozendict import frozendict
 
 if ...

I don't understand why deferring the import until execution time is a Good Thing. Couldn't we just put it at top-of-file, where isort could manage it? Is there really so much overhead that we can't do this at import time? If an app author was willing to endure the overhead of importing this function, then I feel they would also be willing to pull in frozendict at import time.

be explicit

def deepfreeze(
 o,
 ...

This is a nice enough signature, using somewhat conventional parameter names. And it comes with a very nice docstring. But it wouldn't hurt to spell out obj, or perhaps o: object.

lint

The conversion map is in getFreezeConversionMap().

PEP 8 asks that we name it get_freeze_conversion_map().

optional type hints

can also pass a map of custom converters with custom_converters

There's a pair of parameters that default to None, which offers meager clues to humans and to {mypy, pyright} type checkers about what sort of map we might be talking about. Consider adopting mypy --strict. Consider showing a type hint of custom_converters: dict[str, str] or whatever it was you had in mind.

extract helper

The body of def deepfreeze() seems a bit long, in that I cannot view all the source code on my laptop without doing some vertical scrolling. Consider breaking out one or more _private helper functions.

pragma

I very much applaud your efforts to "lint clean!", and for coverage, and so on. It shows admirable dedication to your craft.

I don't get this:

 else: # pragma: no cover
 raise

I mean, the whole DbC thing is brilliant, we have a clear contract, that's all good.

But as far as testing goes? Just provoke that raise, right? Either it's "easy" to execute that line, or we wrote more code than strictly necessary. I would much rather see the automated tests grow by a few lines, than to burden the target code with yet another #pragma.

ancient interpreters

Y'all have been releasing this since 2012, a very impressive track record. I stand in awe. So much has changed, yet the code still works, yay!

There are some places in the GitHub repo where we test for sys.version. Some interpreter versions that are mentioned in your codebase, such as 3.8, have passed end-of-life. I recommend

assert sys.version_info >= (3, 9), "please use a modern interpreter"

at import time, to simplify your life in supporting old stuff.

Also, I feel there's room to make stronger promises about supported interpreters, in the sense of "we will break the build" if 3.9 or a subsequent interpreter fails on an automated test. GitHub Actions offers one approach to routinely running tests under each interpreter as part of the PR cycle. Or uv offers a way to quickly switch between interpreter versions.

modern type hinting

The __init__.pyi is nice enough, and I thank you for it. I'm sure it makes life easier for some folks. But I wonder if maybe we no longer need it in a modern type checking setting? That is, with annotated source code, perhaps we could $ rm *.pyi and linters wouldn't notice? Because they already have what they need?

I have worked with several modern code bases, where $ mypy --strict *.py lints cleanly, and I've found little need for *.pyi files to help with that. If there's an aspect I'm not yet seeing, it might be worth adding a # comment to that file.

• Marco Sulla: I'm forced to add multiple comments. - imports at top: well, I have that import also at top, but for cleaning I delete the imported module at the end. In this way the module can't pollute the autocomplete of an IDE or dir(). - extract helper: I'll think about it, even if I'm reluctant to create a "subfunction" that is used in one place only
• Marco Sulla: - be explicit: well, I like o :D but maybe I can improve the docstring a little - lint: yes, maybe it's better to adhere to PEP 8 - optional type hints: well, the type hints are in init.pyi - There are some places in the GitHub repo where we test for sys.version: well, the only place I use it to stop the code is in run_type_checker.py, that runs a static type checker using typing_extensions.assert_type, that is available only for 3.9+. But it's for testing only.
• Marco Sulla: - modern type hinting: I'm forced to use the .pyi because there's not only the pure py implementation, but a C extension.
• Marco Sulla: Anyway, do you have any suggestion about the correctness of the code? For example, is it good to convert the object in a frozendict if it has a __dict__ attribute? Is it good to consider the object immutable if it's hashable? And so on