Skip to content

Commit

Permalink
[DOC] guide for adding cython based estimators (#4388)
Browse files Browse the repository at this point in the history
This adds a small paragraph to the estimator contribution guide to cover cython based estimators.

Based on the example #4337, and a guide-like summary of the discussion in #4296.

FYI @heerme, @lnthach.
  • Loading branch information
fkiraly committed Apr 2, 2023
1 parent 1329521 commit 346b189
Showing 1 changed file with 23 additions and 0 deletions.
23 changes: 23 additions & 0 deletions docs/source/developer_guide/add_estimators.rst
Expand Up @@ -232,3 +232,26 @@ additional things need to be done:
Don't panic - when contributing to ``sktime``, core developers will give helpful pointers on the above in their PR reviews.

It is recommended to open a draft PR to get feedback early.

Estimators dependent on cython
------------------------------

To add an estimator to ``sktime`` that depends on cython, the following additional steps are needed:

* all cython code should be present in a separate package on ``pypi`` and/or ``conda-forge``.
No cython dependent code should be added directly to ``sktime``.
Below, we call this separate package ``home-package``, for simplicity of reference.
* In ``home-package``, it is recommended to test the estimator via ``check_estimator``,
on the same test matrix as ``sktime``: all supported python versions; MacOS, Linux, Windows.
* In ``sktime``, an interface to the algorithm should be added.
This can be a simple import from ``home-package``,
if the algorithm in ``home-package`` already passes ``check_estimator``.
* Alternatively, the algorithm can be interfaced via a delegator as a delegate,
tags and method overrides can be added in the delegator. See, e.g., ``MySQM`` for this.
* For the ``sktime`` interface, the ``requires_cython`` tag should be set to ``True``,
and the ``python_dependencies`` tag should be set to the string ``"home-package"``.

If all has been setup correctly, the estimator will be tested in ``sktime`` by the
CI element ``test-cython-estimators``.
Note that this CI element does not cover the full test matrix
of python version and operating systems, this should be done in the upstream package.

0 comments on commit 346b189

Please sign in to comment.