Support subclasses in encoders/decoders

[sdhiscocks]

It would be useful to be able to (optionally) automatically support subclasses of any registered class. A simple example of this would be pathlib.Path in the existing examples, where additional registrations are currently required for the concrete types (example 10).

This could be optional, such to provide backward compatibility, and where explicit encoder/decoder is required for a subclass the explicit encoders/decoders take precedence.

[sdhiscocks]

I've opened #46 as some initial draft changes that implement this, and happy to continue developing tests/documentation if there is interest in this feature.

[lchoquel]

Hi @sdhiscocks —thanks a lot for the thoughtful suggestion and for opening PR #46!

Automatic subclass support seems to me totally in the spirit of Kajson.
I like the opt-in flag for backward compatibility. And thank you for updating the example and unit test.

Could you also mention this new feature in the docs?

As soon as it's documented I guess you can make it ready for review (our of Draft status). We can merge and ship this along with PR #43, btw it would be nice if you would review that one #43.

Thanks again for the contribution—really appreciate the initiative!
Let me know if you need any guidance on the docs section.

[bpietropaoli]

Just to add to the conversion, I'd like to make sure I understand what you want to do here:
You'd like to automatically try to use the decoder/encoder from the parent class if it cannot find anything for the current class, is that it?

My concern would be the speed of such a thing. I see in your pull request that you lookup the mro() every time you try to serialise/deserialise. I know speed is not necessarily the main concern here in this library, but if we can prevent running some unnecessary loops in the lookups, that'd be amazing. I used a dictionary to store encoders/decoders for its constant lookup time.

I like the automatic subclass registration though. So I would do it only at the registration level instead of at the lookup time, actually registering the subclasses.

For this, you can use cls.subclasses() recursively to go deeper into the hierarchy. Here is a breadth first version of it:

class Example:
    def __init__(self, a, b):
        self.a = a
        self.b = b

class Subexample(Example):
    def some_method(self):
        print("subexample")

class Subsubexample(Subexample):
    def some_other_method(self):
        print("subsubexample")

class Subsubexample2(Subexample):
    pass

class Subexample2(Example):
    pass


subs = Example.__subclasses__()
all_subs = [] + subs
while subs:
    new_subs = []
    for cls in subs:
        new_subs += cls.__subclasses__()
    all_subs += new_subs
    subs = new_subs

print(all_subs)

So I'd say, actually register the subclasses in _encoders/_decoders. At least, you go through the hierarchy only once.

[sdhiscocks]

Thanks @lchoquel and @bpietropaoli

I'll take a look at adding some docs, and also some additional unit tests, and check it works with #43

at the registration level instead of at the lookup time

Unfortunately this doesn't work if the subclass hasn't been imported already at the time of registration. I'm not sure there is a way to work around this issue.

[sdhiscocks]

@bpietropaoli I've added 8731021, which caches the lookups, which I think should address the main performance issue, and also support post registration imports. (EDIT: and also a417390 to cache case for non-registered classes)

[bpietropaoli]

I understand now. Though, if you add the classes dynamically, I don't see why you wouldn't be able to register them when they are created. In that case, I would look at the mro at the class creation to find the nearest parent with a currently registered decoder/encoder and register it.

And if you don't want to have to do it everywhere in your code, then I would add a method to the UniversalJSONEncoder/UniversalJSONDecoder, taking one argument (the new class) and doing the search for registration, but you would still have to call it every time you create a new class dynamically.

Something in the vein of:

def register_as_nearest_parent(self, obj_type):
    for parent_type in obj_type.mro():
            if parent_type in cls._decoders:
                cls._decoders[obj_type] = cls._decoders[parent_type]
                break
    # Either ignore the fact nothing was found, or raise an exception.

Again, this would do the lookup only once. I don't see why registering on the fly classes created on the fly would be an issue.

[sdhiscocks]

Thanks @bpietropaoli. One issue would be where third party dependencies introduce new subclasses, such have no control of registration at creation time, so it would be good to dynamically support those.

[bpietropaoli]

Alright, can you specify a bit better the use case you have in mind, pls?
I'd like to find a solution that fits your use case and see how we can solve it with the library without changing too many things at once. :)

Which third-party software/libraries are you using? How is it introducing new classes dynamically? How come you don't know of these classes? Because one way I see it too is that if you cannot register immediately, you can always register later. Nothing prevents you from catching the regular exception (TypeError for something non JSON serialisable) raised when you try to encode/decode something that doesn't work.

It looks to me like you're using a meta-programming framework without any control over it, or any information on what it does. That sounds wrong. The idea of the library is that it works for many classes implicitly by using the internals of how python implements objects, and if it doesn't work, you can always either edit your class to include specific dunder methods OR register. But you're asking it to automatically register now. Any class with a default constructor (either an empty constructor or a constructor that simply takes all the attributes values as arguments) will work without any registration required. If your code generator starts creating classes outside of this scope, there is no way you cannot know/catch when it is creating new classes.

In your own code, I would do something in the like:

new_obj = a_new_type

retry = False
try:
    json_str = kajson.dumps(new_obj)
except TypeError:
    UniversalJSONEncoder.register_as_nearest_parent(new_obj.__class__)
    retry = True
if retry:
    json_str = kajson.dumps(new_obj) # Might still not work depending on how register_as_nearest_parent() is implemented

This would automatically register as the nearest parent for anything unknown.
I'd be for raising an exception is register_as_nearest_parent() if no suitable parent could be found. This way, you could even catch that no parent is yet registered. (And if you tell me that the sister class now is not handled, it would handled the same way :p)

Don't forget you can always extend the UniversalJSONEncoder/Decoder and redo the simple interface for load()/dump() using your own encoder/decoder class as done in kajson.py.

I definitely see usages in registering subclasses and registering as nearest parents. I don't see the use case where you can't dynamically register at any time. If you can explicit your use case, I'd better understand the requirements. :)