Skip to content

[BUG,MNT] Fix ClaSP/ACF crash bugs and improve transformations/series test coverage#3533

Open
TonyBagnall wants to merge 8 commits into
mainfrom
ajb/cov2
Open

[BUG,MNT] Fix ClaSP/ACF crash bugs and improve transformations/series test coverage#3533
TonyBagnall wants to merge 8 commits into
mainfrom
ajb/cov2

Conversation

@TonyBagnall

@TonyBagnall TonyBagnall commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Improves test coverage for several modules in aeon.transformations.series
towards 100%, fixing four real bugs uncovered along the way and removing
dead/unreachable code.

Bugs fixed

  • ClaSPTransformer crashes with scoring_metric="F1": _binary_f1_score
    divided by zero (tp / (tp + fp) etc.) whenever a split produced no
    predicted/true positives for a class. Under real numba JIT compilation this
    raised ZeroDivisionError (not just a nan/warning as under plain numpy),
    crashing fit/transform outright on realistic data. Precision/recall are
    now defined as 0.0 when their denominator is zero, matching the standard
    convention for undefined precision/recall.

  • ClaSPTransformer crashes when the series is short relative to the
    window length
    : _compute_distances_iterative's fallback for "fewer than
    k candidate neighbours exist" tried to assign an (n,)-shaped array of
    indices into a (k,)-shaped row when n < k, raising a shape-mismatch
    ValueError. Fixed by cycling through the available indices to fill the
    row instead.

  • AutoCorrelationSeriesTransformer raised the wrong exception type: the
    "lags too large for series length" error message referenced a nonexistent
    self._n_lags attribute, so the intended ValueError was masked by an
    AttributeError instead.

  • STLSeriesTransformer/MSTLSeriesTransformer never used Numba by
    default
    : _resolve_use_numba_local returned bool(self.use_numba),
    which silently turns the documented default use_numba=None ("auto: use
    Numba if available, otherwise NumPy") into False. So the default
    configuration always took the slower NumPy path, even with Numba
    installed. The existing parity test only passed because both backends are
    numerically equivalent by design — it never checked which one actually
    ran. Fixed so None stays None and defers to the existing
    Numba-availability auto-detection. A follow-up issue (#) tracks a
    bigger question this surfaced — whether Numba's optional-import fallback
    in _stl.py still makes sense given it's a core dependency — which is out
    of scope here; _stl.py's own test coverage is intentionally left
    untouched in this PR pending that decision.

Dead/unreachable code removed

  • Unused _sliding_window helper in _clasp.py (not called anywhere).
  • Duplicate elif/exact-complement branches in _yeojohnson.py
    (elif lmbda == 0 after if lmbda != 0, etc.) and _clasp.py's
    _check_scoring_metric — these conditions can never be false given the
    prior branch, so simplified to else.
  • An unreachable else branch in _pla.py's SWAB loop: analytically,
    current_data_point can never exceed len(X) since _best_line never
    returns more elements than remain in the series, so the "data exhausted"
    branch was dead.

Coverage

Brought the following modules to ~96–100% statement coverage (measured with
NUMBA_DISABLE_JIT=1, since coverage.py cannot trace into compiled @njit
code): _collection_wrapper.py, _boxcox.py, _yeojohnson.py,
_scaled_logit.py, base.py, compose/_pipeline.py, compose/_identity.py,
smoothing/_rms.py, _diff.py, _log.py, _warping.py, _dobin.py,
_bkfilter.py, _pla.py, _clasp.py, _mstl.py. Added two new test files
(test_collection_wrapper.py, test_log.py) for previously untested
transformers.

A few branches are intentionally left uncovered: defensive RuntimeError
guards and generic roc_auc-style edge cases that are unreachable given how
they're actually invoked in this codebase, plus code gated behind the
statsmodels/stumpy soft dependencies (not installed in this environment).
_stl.py itself is excluded from the coverage pass for now (see the Numba
bug note above).

@TonyBagnall TonyBagnall requested a review from patrickzib as a code owner June 21, 2026 17:24
@TonyBagnall TonyBagnall added transformations Transformations package coverage Code requiring greater test coverage labels Jun 21, 2026
@aeon-actions-bot aeon-actions-bot Bot added bug Something isn't working maintenance Continuous integration, unit testing & package distribution labels Jun 21, 2026
@aeon-actions-bot

Copy link
Copy Markdown
Contributor

Thank you for contributing to aeon

I have added the following labels to this PR based on the title: [ bug, maintenance ].
I would have added the following labels to this PR based on the changes made: [ transformations ], however some package labels are already present.

The Checks tab will show the status of our automated tests. You can click on individual test runs in the tab or "Details" in the panel below to see more information if there is a failure.

If our pre-commit code quality check fails, please run pre-commit locally and push the fixes to your PR branch.

Don't hesitate to ask questions on the aeon Discord channel if you have any.

PR CI actions

These checkboxes will add labels to enable or disable CI functionality for this PR. This may not take effect immediately, and a new commit may be required to run the new configuration.

  • Run pre-commit checks for all files
  • Run mypy typecheck tests
  • Run all pytest tests and configurations
  • Run all notebook example tests
  • Run numba-disabled codecov tests
  • Disable numba cache loading
  • Regenerate expected results for testing
  • Push an empty commit to re-run CI checks

@patrickzib

Copy link
Copy Markdown
Contributor

@TonyBagnall thank you tony. I will check.

@patrickzib

Copy link
Copy Markdown
Contributor

@TonyBagnall yes, ClaSP changes are fine. Thank you. The ClaSP tests seem to be a bit over-engineered, but are ok.

@TonyBagnall TonyBagnall removed the request for review from patrickzib July 2, 2026 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working coverage Code requiring greater test coverage maintenance Continuous integration, unit testing & package distribution transformations Transformations package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants