-
Notifications
You must be signed in to change notification settings - Fork 31
Open
Description
@anniegbryant, @benfulcher, I would like to congratulate you to this nice package, I really like the concept and it is quite nicely designed! There are also a lot of useful methods collected! Nice.
Now imo the next "big" question is integrability with the wider modelling ecosystem, e.g., can I use the pairwise time series metrics as components in sktime or sklearn. Where with "I", of course, I mean the wider user ecosystem.
Currently, I think there are a few blockers, but would you be interested to resolve them together?
Two main points imo from the codebase review:
sklearninteroperable interfaces expect a few things such as__init__signature related, and availability ofget_params,set_params. You can get this for free by inheriting fromscikit-basebase classes, of course that's not the only way to satisfy the interface requirements.sktimehas related classes which you could adopt or adapt, e.g., theBasePairwiseTransformerPanel. Options could involve, writing an adapter insktime, or using the class inpyspi, the latter would give you testing for free by usingcheck_estimator. Or, writing your own base class template based onscikit-basethat marries the current interface definition withsklearnandsktimeexpectations.
Side points but synergistic points:
- testing could - and should - be more systematic for reliable use, e.g., CI on operating system and python version combinations. Happy to help setting this up if we set aside some time. Of course, the "sktime interface" option would take care of this as part of sktime, although bugfixing could become more clunky as we would have to push bug reports upstream (like in
pycatch22). - a good object/estimator search utility might be nice for the user, there are a lot of implemented objects! We could lift some components from
sktimeorskbasehere.
Metadata
Metadata
Assignees
Labels
No labels