Add support for subclasses with encoder/decoder by sdhiscocks · Pull Request #46 · Pipelex/kajson

sdhiscocks · 2025-07-14T16:59:06Z

Resolves #45

github-actions · 2025-07-14T16:59:14Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

sdhiscocks · 2025-07-14T17:16:15Z

I have read the CLA Document and I hereby sign the CLA

bpietropaoli · 2025-07-15T19:48:50Z

I'm sorry but I would reject this version as it does the lookup for every serialisation/deserialisation.
As I mentioned in the issue conversation, I think you can easily actually register the subclasses by exploring cls.subclasses() recursively. :)

bpietropaoli

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

bpietropaoli · 2025-07-15T20:01:57Z

kajson/json_decoder.py

+        if include_subclasses:
+            UniversalJSONDecoder._multi_decoders[obj_type] = decoding_function
+        else:
+            UniversalJSONDecoder._decoders[obj_type] = decoding_function


Suggested change

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. ;)

bpietropaoli · 2025-07-15T20:02:11Z

kajson/json_decoder.py

    def clear_decoders(cls) -> None:
        """Clear all registered decoders. Primarily for testing purposes."""
        cls._decoders.clear()
+        cls._multi_decoders.clear()


Suggested change

cls._multi_decoders.clear()

bpietropaoli · 2025-07-15T20:02:45Z

kajson/json_decoder.py

+        """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


Suggested change

"""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

bpietropaoli · 2025-07-15T20:03:46Z

kajson/json_decoder.py

+    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


Suggested change

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)

bpietropaoli · 2025-07-15T20:10:04Z

kajson/json_encoder.py

+        if (func := self.get_registered_encoder(obj_class)) is not None:
            try:
-                the_dict = UniversalJSONEncoder._encoders[type(obj)](obj)
+                the_dict = func(obj)


Suggested change

the_dict = func(obj)

the_dict = UniversalJSONEncoder._encoders[type(obj)](obj)

bpietropaoli · 2025-07-15T20:10:21Z

kajson/json_encoder.py

+                func_name = func.__name__
+                error_msg = f"Encoding function {func_name} used for type '{obj_class}' raised an exception: {exc}."


Suggested change

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}."

bpietropaoli · 2025-07-15T20:10:38Z

kajson/json_encoder.py

                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."


Suggested change

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."

bpietropaoli · 2025-07-15T20:10:57Z

kajson/json_encoder.py

        # 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}")


Suggested change

raise TypeError(f"Type {obj_class} is not JSON serializable. Value: {obj}")

raise TypeError(f"Type {type(obj)} is not JSON serializable. Value: {obj}")

bpietropaoli · 2025-07-15T20:12:37Z

tests/integration/test_encoder_registration.py

The new feature should include a unit test too, testing this functionality in particular.

Co-authored-by: Bastien Pietropaoli <bastien.pietropaoli@gmail.com>

Add support for subclasses with encoder/decoder

118f745

sdhiscocks mentioned this pull request Jul 14, 2025

Support subclasses in encoders/decoders #45

Open

bpietropaoli suggested changes Jul 15, 2025

View reviewed changes

sdhiscocks and others added 5 commits July 16, 2025 08:00

Add unit tests for subclass support in encoder/decoder

fa30611

Improve doc string for subclasses in encoder/decoder

046ff3a

Co-authored-by: Bastien Pietropaoli <bastien.pietropaoli@gmail.com>

Fix Decoder type definition for Python 3.9

0e44300

Add caching of encoder/decoder function lookup

8731021

Also cache missing case in encoder/decoder

a417390

-        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

	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}."

	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."

	raise TypeError(f"Type {obj_class} is not JSON serializable. Value: {obj}")
	raise TypeError(f"Type {type(obj)} is not JSON serializable. Value: {obj}")

Conversation

sdhiscocks commented Jul 14, 2025

Uh oh!

github-actions bot commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sdhiscocks commented Jul 14, 2025

Uh oh!

bpietropaoli commented Jul 15, 2025

Uh oh!

bpietropaoli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Jul 14, 2025 •

edited

Loading