Plan for new way of multiple dispatch in orekit python wrapper

Hi,

For the v12.0 orekit I have a suggestion for a new way of managing overloading/dispatch. In java there is possibility to have several methods with different types and same name while this is not possible in python in same way. Note that this is only a topic when subclassing java classes in Python, so quite advanced usage.

Previously this has been solved by adding some extra characters to the method names highlighting the different types like _FFA for three parameters. This gives clear entry point for each java method in python and ensures that all combinations are implemented in the subclass.

However, when working with orekit v12 it becomes clear that this is quite a quirky method when we have methods with lots of overloads.

With some tests confirming that it works, I am suggesting that all these overloaded methods will call same ptyhon method and that that method needs to take care of knowing which type of response is expected. This is a bit of overhead in the python class, but i find it less quirky and more clean.

For example:

class TestGroundPointing(PythonGroundPointing):
    def getTargetPV(self, pvProv, date, frame) -> TimeStampedPVCoordinates|TimeStampedFieldPVCoordinates:
        if isinstance(pvProv, FieldPVCoordinatesProvider):
            return TimeStampedFieldPVCoordinates(date, FieldPVCoordinates.getZero(date.getField()))

        elif isinstance(pvProv, PVCoordinatesProvider):
            return TimeStampedPVCoordinates(date, PVCoordinates.ZERO)

        else:
            raise RuntimeError(f'Not supported type of PVCoordinatesProvider: {type(pvProv).__name__}')

There are 3rd party libraries to support this type of problems as well, like multipledispatch which gives a quite clean code:

from multipledispatch import dispatch

class TestGroundPointing(PythonGroundPointing):
    @dispatch(PVCoordinatesProvider, AbsoluteDate, Frame)
    def getTargetPV(self, pvProv, date, frame) -> TimeStampedPVCoordinates|TimeStampedFieldPVCoordinates:
        return TimeStampedPVCoordinates(date, PVCoordinates.ZERO)

    @dispatch(FieldPVCoordinatesProvider, FieldAbsoluteDate, Frame)
    def getTargetPV(self, pvProv, date, frame) -> TimeStampedPVCoordinates|TimeStampedFieldPVCoordinates:
        return TimeStampedFieldPVCoordinates(date, FieldPVCoordinates.getZero(date.getField()))

In previous versions this has been implemented by a second method in the Python class called “getTargetPV_FFF” for the Fields version.

Let me know if you have any thoughts or issues with this change?

Hi Petrus,

For starters, thanks a lot, again, for taking care singlehandedly and so swiftly of the Python wrapper. And thank you also for being proactive on its improvement.

I’m guessing your proposition would mean that performance could take a hit in some situations, as IF/ELSE conditions are not so fast to evaluate in non-precompiled languages. I know it’s only when making custom inheritors of Orekit stuff, but some classes are designed for that like AdditionalDerivativesProvider.

On the other hand, I do understand that the fact that Java can have different signatures with same name while Python cannot is a real problem for maintenance. So it really depends on how much of a nightmare it is for you to keep going with the current solution of creating more methods with weird suffixes…

Using the dispatch package might be problematic though, as it introduces a dependency on a third party, whereas the wrapper has been pretty much self contained so far (I mean excluding the JVM and black magic to expose it to Python).

Cheers,
Romain

Thanks Romain for the response and kind words, and yes it would likely have a slight impact on performance with some additional if statements. One can try to optimize the order of the statements in likelyhood.
But the real issue is as you point out the maintainability and “least surprise” on what these additional methods are called. I think this last is probably the most critical, where one today basically needs to read the wrapper code to get the naming right - there is also a sometime tricky decision which is the “prime” method (not to be renamed) and which are “overloaded”. If going for the renaming method consistent and error-free I think one should make some automatic code generation stuff…

I am all agreeing with not introducing more package in core orekit wrapper. Using additional packages for this would be up to the user, not part of the orekit package itself. I have found that it is also possible to use the new match statement in python 3.10 and above, which probably is the neatest solution:

class TestGroundPointing(PythonGroundPointing):
    def getTargetPV(self, pvProv, date, frame) -> Union[TimeStampedPVCoordinates,TimeStampedFieldPVCoordinates]:
        match (pvProv):
            case FieldPVCoordinatesProvider():
                return TimeStampedFieldPVCoordinates(date, FieldPVCoordinates.getZero(date.getField()))
            
            case PVCoordinatesProvider():
                return TimeStampedPVCoordinates(date, PVCoordinates.ZERO)

            case _:
                raise RuntimeError(f'Not supported type of PVCoordinatesProvider: {type(pvProv).__name__}')


Regards!

1 Like

Hi Petrus,

Great if there is a native way of handling this with recent Python.
I’d say go ahead with your proposition.

Cheers,
Romain.