[BUG] ShapeletTransformPyts().fit(x) throws "ValueError: ShapeletTransformPyts requires y in fit."

[navaldroid]

Describe the bug

For some reason, ShapeletTransformPyts (and also ShapeletTransform) requires the y parameter in their fit() method, despite the documentations for fit(X, y=None) reports it as optional

import numpy as np
from sktime.transformations.panel.shapelet_transform import ShapeletTransformPyts

x = np.random.random(10000)
model = ShapeletTransformPyts()
fitted = model.fit(x)

>>> ValueError: ShapeletTransformPyts requires `y` in `fit`.

Versions
Python 3.10.10
numpy==1.26.4
pyts==0.13.0
sktime==0.29.0
scikit-base==0.7.7

[fkiraly]

This is not a bug, but possibly a documentation improvement point.

ShapeletTransformPyts indeed requires y in fit. This is as in the interfaced transformer, from pyts, see https://pyts.readthedocs.io/en/stable/generated/pyts.transformation.ShapeletTransform.html

The documentation could be clearer - some transformations do require y in fit, some do not. The current docstring says "some transformers require this, see class docstring for details".

Do you have a suggestion how this could be improved? Perhaps pointing to the requires_y tag?

[navaldroid]

Thanks for your reply. Sure, I will update the documentation, just removing the "=None" or "optional" where needed.

…

On Fri, May 3, 2024 at 11:24 AM Franz Király ***@***.***> wrote: This is not a bug, but possibly a documentation improvement point. ShapeletTransformPyts indeed requires y in fit. This is as in the interfaced transformer, from pyts, see https://pyts.readthedocs.io/en/stable/generated/pyts.transformation.ShapeletTransform.html The documentation could be clearer - some transformations do require y in fit, some do not. The current docstring says "some transformers require this, see class docstring for details". Do you have a suggestion how this could be improved? Perhaps pointing to the requires_y tag? — Reply to this email directly, view it on GitHub <#6379 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGAFD4SKDTMUPYJTE5IKONLZANJTVAVCNFSM6AAAAABHD7G33CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJSGYZTCNJWGM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[fkiraly]

@navaldroid, thanks for your help - although I do not think it is as easy - or unproblematic - to remove the default.

The default needs to be there in fit, because some transformations do not require y.

The docstring also applies to all transformations at once, at the moment - so a change there would make it false for the remaining transformer.s