[ENH] adapter for scipy distributions

[fkiraly]

Adapting scipy distributions is very formulaic and could easily be dealt with by an adapter class. This also avoids duplication in implementation efforts, as scipy is a core dependency.

This may require some abstraction around methods, but it seems mostly like a delegator class, as scipy does its own broadcasting.

Further discrepancies to be mindful of:

• energy and similar integrals are not implemented in scipy
• the class parameterization is different, scipy uses class methods whereas skpro uses __init__ based parameterization

Good first issue with a design/architecture flavour, can be leveraged to cover a lot of ground in #22.

[malikrafsan]

Hi @fkiraly I am very interested in being assigned to this issue!

My solution to "the class parameterization" is that we store the parameter of the class and then pass it to the methods. I have created a preliminary solution for this. I have compared it to the existing distribution class and it has a similar result

Regarding the "energy and similar integrals are not implemented in scipy", I haven't found any solution for this, do you have any suggestions? Thank you so much!

[fkiraly]

Yes, this is exactly what I meant - although on an adatper level, rather than for each distribution. That is, inheriting from the adapter and putting the scipy class in a field should do the job.

Similar to _PytsAdapter and _TslearnAdapter in sktime.

[malikrafsan]

Hi @fkiraly , firstly I want to apologize for my recent absence from SKPro project. I have quite a lot of college assigments and exams previously and unfortunately, I wasn't able to contribute as much as I would have liked 😅. However, currently there is not a lot of things on my way and I am eager to continue contributing 😅

Similar to _PytsAdapter and _TslearnAdapter in sktime.

I will check those two ASAP. Looking forward to catching up and contributing!

[malikrafsan]

What I understand from _PytsAdapter and _TslearnAdapter is that we create an Adapter Class that will be a base class for wrapping another library functionality.

We create the base class to have some methods that the child classes can override by returning the specific class/function from the library that we want to wrap

# _PytsAdapter
    def _get_pyts_class(self):
        raise NotImplementedError("abstract method")

# RocketPyts
    def _get_pyts_class(self):
        from pyts.transformation.rocket import ROCKET
        return ROCKET

We then initialize the class and store it on the attribute. When we want to the methods needed, we use that initialized attribute and use the functionality

# _PytsAdapter
    def _predict(self, X, y=None):
        pyts_est = getattr(self, self._estimator_attr)
        return self._call_with_y_optional(pyts_est.predict, X, y)

What I am confused about is that, the _PytsAdapter and _TslearnAdapter initialized the estimator attribute on the _fit method. But in the scipy adapter, there is no method bound to be called first (unlike _fit method). Do we need to move the initialization on the constructor method for scipy adapter?

I will try to raise a draft PR containing my proposed adapter ASAP

[fkiraly]

Welcome back, @malikrafsan!

Generally, no need to apologize - as long as you are not paid or have not otherwise committed, there is no expectation that you deliver. Most contributions are on a volunteer or reseach basis, so there is absolutely no pressure. Thanks a lot for your previous work!

Re the adapter, the main idea would be avoiding duplication in classes interfacing scipy classes - these are already vectorized methods, but do not store parameters. The design principle here is being "DRY" - don't repeat yourself.

The precise way to achieve this depends on the inner and outer interface of the adapter, so pyts / tslearn adapters are not 100% comparable - as you say, there is no fit in scipy distributions.

So, one would have to design something where the scipy distribution, with some information about parameter names, sits inside the adapter.

What tends to be helpful is to interface a few instances and then abstract from them:

• one example, and the one I would use as a template, is the Fisk distribution (Fisk).
• similar to that, sort of a first try, is Poisson. This will need to be refactored.

My strategy would be: start with Fisk and try to make it more general. Make a copy and turn it into an adapter class. Then have Fisk inherit from the adapter, and simplify as much as possible. Then create a second distribution, and then refactor Poisson.

[malikrafsan]

Hi @fkiraly, thank you so much for your understanding!

I have created a new draft PR containing the proposed base Scipy adapter class and inherited adapter class for the Fisk and Poisson distributions here. Please let me know your thoughts about it, I would be more than happy to get your feedback!

Once again, thank you so much for having me!