Skip to content

repeated_median_slope silently drops NaN pairs and leaks RuntimeWarnings #558

Description

@kgdunn

What happens

repeated_median_slope (regression/_robust_regression.py:31) has no NaN handling. A missing value in y is silently dropped and a slope is returned from the surviving pairs, with a RuntimeWarning as the only signal:

from process_improve.regression._robust_regression import repeated_median_slope
import numpy as np

repeated_median_slope(np.array([0., 1, 2, 3]), np.array([5., 5.5, 6, 6.5]))   # 0.5   (clean)
repeated_median_slope(np.array([0., 1, 2, 3]), np.array([5., np.nan, 6, 72])) # 33.25 (one NaN)
repeated_median_slope(np.array([0., 1, 2, 3]), np.array([np.nan]*4))          # nan

The one-NaN call emits RuntimeWarning: All-NaN slice encountered; the all-NaN call emits five of them.

Why

for i in np.arange(len(x)):
    inner_medians = []
    for j in np.arange(len(y)):
        den = x[j] - x[i]
        if j != i and den != 0:
            inner_medians.append((y[j] - y[i]) / den)

    medians.append(np.nanmedian(inner_medians))     # :91

return np.nanmedian(medians)                        # :93

When y[i] is NaN, every slope out of point i is NaN, so np.nanmedian receives an all-NaN list: it warns and returns NaN. The outer np.nanmedian then drops those NaN entries, so the result is computed from whichever points happened to be clean. Nothing in the signature, the docstring, or the return value records that observations were discarded.

The docstring documents the np.nan return only for the all-equal-x case:

Returns np.nan if all inner medians are undefined (e.g. all x values are equal).

Why it matters beyond this function

ControlChart._holt_winters_warmup_fit calls it for beta_0 (monitoring/control_charts.py:355), which is where the leaked All-NaN slice encountered warning surfaces for a user who never named this function. That path is the subject of the zero-width-control-limits defect filed alongside this one.

Suggested fix

  • Mask to the finite pairs explicitly instead of relying on np.nanmedian to mop up, so no all-NaN slice is ever constructed and no RuntimeWarning escapes.
  • Raise ValueError when fewer than three finite pairs remain, consistent with the existing len(x) <= 2 guard at :72-73 and with docs/development/error_handling.rst case 1.
  • Document the omission in the docstring, so dropping missing observations is a stated contract rather than an accident of np.nanmedian.

A nan_policy parameter (scipy convention, already used elsewhere in this repo via median_absolute_deviation(..., nan_policy="omit")) would make the choice explicit and selectable, but that is an API addition and is deliberately left out of the bug fix.

Acceptance criteria

  • No RuntimeWarning escapes for any NaN input.
  • Fewer than three finite pairs raises ValueError with a message naming the count.
  • The clean-input result is unchanged (0.5 for the example above).
  • The NaN-omission behaviour is documented and tested.

Provenance

The corresponding # TODO is tests/test_regression.py:25 ("handle cases where there are nans in the vectors"), currently rolled up in #213. Found during a TODO/FIXME audit.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions