Protobuf converting a class inheriting up model classes

[Cryoscopic-E]

Describe the bug
KeyError when using the protobuf converter when defining a class derived from InstantaneousAction.

Example:

File "<..>\unified_planning\grpc\converter.py", line 36, in convert
    f = self.functions[type(element)]
        ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^
KeyError: <class 'up_types.actions.sequential.drive_harv_to_field_and_init.ActionDriveHarvToFieldAndInit'>

In this case ActionDriveHarvToFieldAndInit inherits from InstantaneousAction.

To Reproduce
Steps to reproduce the behavior:

1. Create a class that inherits from InstantaneousAction
2. Use the ProtobufWriter convert, to convert the up problem.

Expected behavior
The problem is correctly converted to the protobuf representation.

I've localized the problem in the converter.py file, a simple modification like the following will convert the problem using the base class:

def convert(self, element, *args):

        if type(element) not in self.functions.keys():
            f = self.functions[type(element).__bases__[0]]
        else:
            f = self.functions[type(element)]
        return f(element, *args)

[arbimo]

Thank you for the very clear report. I am not sure I agree with the expected behavior though. In general, the converter can not exactly converter your specialized class. By definition your specialized class will have something more than InstantaneousAction that the converter will not know how to encode (in fact it wont even be aware of its existence).

There are some use case were it could make sense, e.g. if the class just add methods for syntactic sugar. But I am not sure its worth introducing surprising/inconsistent behaviors for them.
What is your use case exactly?

[Cryoscopic-E]

Fair point. I'm converting the model for the agricultural use case, and yes the instance here is about adding methods to a class to be used elsewhere.

[arbimo]

Having thought a bit more about this, I think it would be reasonable to support this use case. While I still agree with my former self, I also think it makes sense to align the planning-through-protobuf part of UP with the planning-through-python part. And indeed using subclasses of InstantaneousAction or others is transparent there.

I won't have time to do it soon, but if a PR was to be submitted I would be happy to review it =)
Beyond what is in the original message, I think it simply needs

1. to be a bit more general (indirect inheritance, ...)
2. to have a test case for it
3. to be careful about error message (the class of the original object should appear in the error message, not the one of an ancestor class)