sktime / sktime

A unified framework for machine learning with time series
https://www.sktime.net
BSD 3-Clause "New" or "Revised" License
7.98k stars 1.38k forks source link

[ENH] rename `annotation` module to `detection` #7295

Open fkiraly opened 1 month ago

fkiraly commented 1 month ago

We should rename the annotation module to detection.

This was suggested by @tveten, and I agree with the rationale, "detection" is a much more common and recognizable term for the learning tasks the module is dealing with, e.g., anomaly and changepoint detection, and segmentation (aka anomalous segment detection, set anomaly detection, if unsupervised).

Further, "annotation" is overloaded, especially due to the "human annotation" term commonly used for human AI labellers, and much less so anymore in, say, "image annotation" for the automated task.

The proposed deprecation strategy is as follows:

  1. map all imports to detection module, while leaving the original module - https://github.com/sktime/sktime/pull/7294
  2. move content from annotation to detection, reverse import direction
  3. after an extended period of warnings, remove the annotation module (perhaps only in the major release 1.0.0)

Discussions appreciated - FYI @Alex-JG3, @sktime/core-developers

yarnabrina commented 1 month ago

A vague opiionated comment: I'd like to request for consideration to find a more descriptive name, as detection is very generic. Apart from anomaly detection, "detection of trend" or "detection of seasonality patterns" are also quite common in time series context, so just "detection" may not be enough. I do not have any alternatives to propose at the moment.


after an extended period of warnings, remove the annotation module (perhaps only in the major release 1.0.0)

Another opinionated comment: it is already openly documented as an experimental module, so I think waiting for major is not necessary, unless you have a plan to do it soon (next 2-3 months). It surely needs longer deprecation cycle than just 2, but not necessarily a full major release.

fkiraly commented 1 month ago

A vague opiionated comment: I'd like to request for consideration to find a more descriptive name, as detection is very generic.

Some genericity is intentional, as it has to be an umbrella for various things like anomaly detection, outlier detection, segmentation, changepoint detection. If you have something better, I am all ears. Also FYI @Alex-JG3, @tveten.

I think it is strictly and significantly better than annotation, and the margin beats the weight of keeping the old name for downwards compatibility's sake.

I also think all tasks should be in a single module as they will share base classes, API elements, and utilities.

Another opinionated comment: it is already openly documented as an experimental module, so I think waiting for major is not necessary,

I would say, the rename and copy can happen immediately, and removal only happens with major.

How would you structure the policy alternatively - 3 minor cycles and then remove?

fkiraly commented 1 month ago

Regarding alternatives, @tveten in skchange is using anomaly_detectors and change_detectors, which also implies the alternative detectors. https://github.com/NorskRegnesentral/skchange/tree/main/skchange

To comment on that:

yarnabrina commented 1 month ago

I think it is strictly and significantly better than annotation, and the margin beats the weight of keeping the old name for downwards compatibility's sake.

I do agree that detection is better than annotation, with respect to more intuitive/common name wise.

fkiraly commented 1 month ago

ok, I also think this is better than letting it as it is. I am going to proceed with the rename then - if the majority of devs feels strongly about this, we can always say it was an experimental module anyway and change.