Skip to content

filter() mutates the explainer - leaks into to_pandas(), plots, to_smartpredictor() #752

Description

@milton-minervino

Summary

SmartExplainer.filter() stores its result on the explainer (self.mask,
self.masked_contributions, self.mask_params).

Two places call filter() without the user asking:

  1. plot.local_plot() calls it with max_contrib=20.
  2. The web app calls it on every move of the "max contributions" slider.

Because the result is stored, the effect does not stop at the plot. It changes what
to_pandas() returns afterwards, it changes every plot drawn afterwards, and it is copied
into the object returned by to_smartpredictor().

Nothing warns the user.

Environment

shapash 2.9.0
Python 3.13.13
scikit-learn 1.8.0
pandas 3.0.5

Reproduction 1 — drawing a chart changes to_pandas()

Two identical explainers. The only difference is that one draws a chart first.

import numpy as np, pandas as pd
from sklearn.ensemble import RandomForestClassifier
from shapash import SmartExplainer

def make_explainer():
    rng = np.random.default_rng(0)
    n = 120
    X = pd.DataFrame({f"f{i}": rng.normal(size=n) for i in range(25)})
    y = pd.Series((X["f0"] + rng.normal(size=n) * 0.3 > 0).astype(int), name="y")
    model = RandomForestClassifier(n_estimators=20, random_state=0).fit(X, y)
    xpl = SmartExplainer(model=model)
    xpl.compile(x=X, y_pred=pd.Series(model.predict(X), index=X.index, name="pred"))
    return xpl

a = make_explainer()
print("no plot    -> features in to_pandas():", (a.to_pandas().shape[1] - 1) // 3)

b = make_explainer()
b.plot.local_plot(index=0)          # draw one chart, nothing else
print("after plot -> features in to_pandas():", (b.to_pandas().shape[1] - 1) // 3)
print("mask_params:", b.mask_params)

Output:

no plot    -> features in to_pandas(): 25
to_pandas params: {'features_to_hide': None, 'threshold': None, 'positive': None, 'max_contrib': 20}
after plot -> features in to_pandas(): 20
mask_params: {'features_to_hide': None, 'threshold': None, 'positive': None, 'max_contrib': 20}

Drawing one chart removed 5 features from the exported dataframe.

Reproduction 2 — a web app slider reaches the user's object and the predictor

SmartApp keeps the explainer by reference. Its callback calls filter() on it. This snippet
runs the same call the slider runs.

xpl = make_explainer()
xpl.filter(max_contrib=2)                 # what the slider triggers

print("features in to_pandas():", (xpl.to_pandas().shape[1] - 1) // 3)
print("SmartPredictor.mask_params:", xpl.to_smartpredictor().mask_params)

Output:

features in to_pandas(): 2
SmartPredictor.mask_params: {'features_to_hide': None, 'threshold': None, 'positive': None, 'max_contrib': 2}

So a slider position in a browser ends up inside the object you deploy.

The web app serves through wsgiref.simple_server, which is single threaded. Requests are
therefore handled one at a time and this is not a data race today. But the state stays between
requests, so a second user inherits the first user's filter. SmartApp also exposes
self.server as a plain Flask app. Deployed under gunicorn with more than one worker, this
becomes a real race on shared state.

Expected behaviour

Drawing a chart should not change the explainer.

Calling filter() explicitly may keep storing its settings, since that is documented and
to_pandas() relies on it. Only the implicit calls are the problem.

Where it happens

Writes:

  • shapash/explainer/smart_explainer.py:946-955filter() assigns mask,
    masked_contributions, mask_params.

Implicit callers:

  • shapash/explainer/smart_plotter.py:307local_plot() calls
    self._explainer.filter(max_contrib=20, display_groups=display_groups).
  • shapash/webapp/smart_app.py:2513 — the Dash callback calls self.explainer.filter(...),
    then reads the result back through local_plot().

Deliberate caller, not part of this bug:

  • shapash/explainer/smart_explainer.py:1179to_pandas().

Readers of the stored state:

  • shapash/explainer/smart_plotter.py:144-147_apply_mask_one_line reads explainer.mask.
  • shapash/explainer/smart_plotter.py:175-179_check_masked_contributions reads
    explainer.masked_contributions.
  • shapash/explainer/smart_plotter.py:352-353 — reads mask_params["positive"].
  • shapash/explainer/smart_explainer.py:1163-1192to_pandas() reads mask and mask_params.
  • shapash/explainer/smart_explainer.py:1601-1603to_smartpredictor() copies mask_params.

The comment at smart_plotter.py:296 states the situation directly:
# If the filter method has not been called yet. Plot output depends on what was called before.

Suggested fix

filter() is a pure computation. It derives everything from data and its four parameters, then
assigns the result. Nothing needs the result to live on the explainer.

  1. Extract a pure function, next to the existing helpers in shapash/manipulation/mask.py:

    compute_mask(state, data, features_to_hide, threshold, positive, max_contrib)
        -> (mask, masked_contributions, mask_params)
  2. Rewrite filter() as a call to it plus the three assignments. Behaviour and public API stay
    the same.

  3. Give local_plot() an optional mask_state parameter. When it is passed, use it. When it is
    not, read from the explainer as today. Pass it down to _apply_mask_one_line and
    _check_masked_contributions instead of having them read self._explainer.

  4. At smart_plotter.py:307, call compute_mask(...) locally instead of filter(). Same
    numbers, no assignment.

  5. In the web app callback, compute the mask and pass it to local_plot().

This is backward compatible. filter() keeps working as documented. Only the implicit
mutations disappear.

Suggested regression test: draw every plot on a compiled explainer and assert that no new
attribute appears on it.

Impact

  • to_pandas() can silently return fewer features than the model uses, depending on whether a
    chart was drawn first.
  • Plot output depends on call order, so notebooks are not reproducible when cells are re-run in a
    different order.
  • Web app users share one filter state.
  • A display setting is copied into SmartPredictor, which is meant for production.

Activity

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

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions