Add support for subclasses with encoder/decoder #46
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
I'm sorry but I would reject this version as it does the lookup for every serialisation/deserialisation. |
bpietropaoli
left a comment
bpietropaoli
left a comment
There was a problem hiding this comment.
If you follow my suggestions, I've basically reverted almost all your changes but added an actual registration of subclasses with the same decoder/encoder directly in the registration method so the hierarchy is search only once at the registration and not at every serialisation/deserialisation.
It should do the same thing, just cleaner (in my humble opinion).
kajson/json_decoder.py
Outdated
| if include_subclasses: | ||
| UniversalJSONDecoder._multi_decoders[obj_type] = decoding_function | ||
| else: | ||
| UniversalJSONDecoder._decoders[obj_type] = decoding_function |
There was a problem hiding this comment.
| if include_subclasses: | |
| UniversalJSONDecoder._multi_decoders[obj_type] = decoding_function | |
| else: | |
| UniversalJSONDecoder._decoders[obj_type] = decoding_function | |
| UniversalJSONDecoder._decoders[obj_type] = decoding_function | |
| # Explore the subclasses tree: | |
| if include_subclasses: | |
| subclasses = obj_type.__subclasses__() | |
| all_subclasses = [] + subclasses | |
| while subclasses: | |
| new_subclasses = [] | |
| for cls in subclasses: | |
| new_subclasses += cls.__subclasses__() | |
| all_subclasses += new_subclasses | |
| subclasses = new_subclasses | |
| for cls in all_subclasses: | |
| UniversalJSONDecoder._decoders[cls] = decoding_function |
By doing so, every other change becomes obsolete. It does the lookup at the registration time only, and actually registers the subclasses. ;)
| def clear_decoders(cls) -> None: | ||
| """Clear all registered decoders. Primarily for testing purposes.""" | ||
| cls._decoders.clear() | ||
| cls._multi_decoders.clear() |
There was a problem hiding this comment.
| cls._multi_decoders.clear() |
| """Check if a decoder is registered for the given type.""" | ||
| if obj_type in cls._decoders: | ||
| return True | ||
| for obj_subtype in obj_type.mro(): | ||
| if obj_subtype in cls._multi_decoders: | ||
| return True | ||
| return False |
There was a problem hiding this comment.
| """Check if a decoder is registered for the given type.""" | |
| if obj_type in cls._decoders: | |
| return True | |
| for obj_subtype in obj_type.mro(): | |
| if obj_subtype in cls._multi_decoders: | |
| return True | |
| return False | |
| """Check if a decoder is registered for the given type. Primarily for testing purposes.""" | |
| return obj_type in cls._decoders |
| def get_registered_decoder(cls, obj_type: Type[Any]) -> Decoder | None: | ||
| """Get the registered decoder for the given type.""" | ||
| if obj_type in cls._decoders: | ||
| return cls._decoders[obj_type] | ||
| for obj_subtype in obj_type.mro(): | ||
| if obj_subtype in cls._multi_decoders: | ||
| return cls._multi_decoders[obj_subtype] | ||
| return None |
There was a problem hiding this comment.
| def get_registered_decoder(cls, obj_type: Type[Any]) -> Decoder | None: | |
| """Get the registered decoder for the given type.""" | |
| if obj_type in cls._decoders: | |
| return cls._decoders[obj_type] | |
| for obj_subtype in obj_type.mro(): | |
| if obj_subtype in cls._multi_decoders: | |
| return cls._multi_decoders[obj_subtype] | |
| return None | |
| def get_registered_decoder(cls, obj_type: Type[Any]) -> Callable[[Dict[str, Any]], Any] | None: | |
| """Get the registered decoder for the given type. Primarily for testing purposes.""" | |
| return cls._decoders.get(obj_type) |
| if (func := self.get_registered_encoder(obj_class)) is not None: | ||
| try: | ||
| the_dict = UniversalJSONEncoder._encoders[type(obj)](obj) | ||
| the_dict = func(obj) |
There was a problem hiding this comment.
| the_dict = func(obj) | |
| the_dict = UniversalJSONEncoder._encoders[type(obj)](obj) |
| func_name = func.__name__ | ||
| error_msg = f"Encoding function {func_name} used for type '{obj_class}' raised an exception: {exc}." |
There was a problem hiding this comment.
| func_name = func.__name__ | |
| error_msg = f"Encoding function {func_name} used for type '{obj_class}' raised an exception: {exc}." | |
| func_name = UniversalJSONEncoder._encoders[type(obj)].__name__ | |
| error_msg = f"Encoding function {func_name} used for type '{type(obj)}' raised an exception: {exc}." |
| pass | ||
| except Exception as exc: | ||
| error_msg = f"Method __json_encode__() used for type '{type(obj)}' raised an exception." | ||
| error_msg = f"Method __json_encode__() used for type '{obj_class}' raised an exception." |
There was a problem hiding this comment.
| error_msg = f"Method __json_encode__() used for type '{obj_class}' raised an exception." | |
| error_msg = f"Method __json_encode__() used for type '{type(obj)}' raised an exception." |
| # If nothing worked, raise an exception like the default JSON encoder would: | ||
| if not already_encoded: | ||
| raise TypeError(f"Type {type(obj)} is not JSON serializable. Value: {obj}") | ||
| raise TypeError(f"Type {obj_class} is not JSON serializable. Value: {obj}") |
There was a problem hiding this comment.
| raise TypeError(f"Type {obj_class} is not JSON serializable. Value: {obj}") | |
| raise TypeError(f"Type {type(obj)} is not JSON serializable. Value: {obj}") |
There was a problem hiding this comment.
The new feature should include a unit test too, testing this functionality in particular.
Co-authored-by: Bastien Pietropaoli <[email protected]>
2 participants
Resolves #45