Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ Bugfixes
- An error that happened when running ``TableReport`` or ``column_associations``
on some dataframes with non-string column names has been fixed in :pr:`2179`
by :user:`Jérôme Dockès <jeromedockes>`.
- An error that could arise in histograms when running :class:`TableReport` on
data with a very small range (less than 10 representable floating-point
numbers between min and max) has been fixed.
:pr:`2189` by :user:`Jérôme Dockès <jeromedockes>`.

Deprecations
------------
Expand Down
53 changes: 49 additions & 4 deletions skrub/_reporting/_plotting.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,14 @@ def _adjust_fig_size(fig, ax, target_w, target_h):


def _get_range(values, frac=0.2, factor=3.0):
if np.issubdtype(values.dtype, np.floating):
finite_values = values[np.isfinite(values)]
else:
finite_values = values
if not len(finite_values):
return 0, 0
min_value, low_p, high_p, max_value = np.quantile(
values, [0.0, frac, 1.0 - frac, 1.0]
finite_values, [0.0, frac, 1.0 - frac, 1.0]
)
delta = high_p - low_p
if not delta:
Expand All @@ -193,7 +199,40 @@ def _get_range(values, frac=0.2, factor=3.0):
return low, high


def _get_safe_hist_range(values):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you explain this function in a bit more detail? it's not clear what is happening in the code and what is being returned

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep i added comments LMK if it is clearer now

# Get a safe min, max range for the histogram (the 'range' parameter of
# np.histogram).
#
# This handles a corner case where the range of the data is very narrow and
# there are less than 10 (the default number of bins) representable
# floating-point numbers between the data min and max (with the precision
# of the input column dtype). In this case numpy/matplotlib histogram with
# default parameters fails, so we pass the 'range' parameter to extend it
# slightly beyond the data range so that it is wide enough to contain 10
# bins.
if not len(values) or not (
np.issubdtype(values.dtype, np.floating)
or np.issubdtype(values.dtype, np.integer)
):
# empty or non-numeric inputs (datetimes) need no special handling.
return None
vmin, vmax = values.min(), values.max()
delta = max(np.spacing(vmin), np.spacing(vmax))
if vmax - vmin > 12 * delta:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why 12 here and 6 below?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the range is already wider than 12, we don't need to adjust it. otherwise we add 6 on each side so it is at least 12-wide afterwards.

probably 5 + 5 = 10 would be enough because there are 10 bins, but we don't care and have a little safety margin to make sure we don't crash

# Min and max are far enough to divide the data range into 10 bins, we
# can keep the default range=None.
#
# 12 is a bit conservative, 10 is probably enough, but there is no
# harm in having the plot range slightly wider than is strictly
# required.
return None
# Extend the range by 12 spacing steps (6 on each side) so the new range
# can contain the bins.
return vmin - 6 * delta, vmax + 6 * delta


def _robust_hist(col, ax=None, color=None):
result = {}
col = sbd.drop_nulls(col)
if sbd.is_float(col):
# avoid any issues with pandas nullable dtypes
Expand All @@ -211,19 +250,25 @@ def _robust_hist(col, ax=None, color=None):
np_histogram_values = sbd.to_numpy(
_datetime_encoder.DatetimeEncoder(resolution=None).fit_transform(col)
).ravel()
result["total_seconds_offset"] = np_histogram_values.min()
np_histogram_values = np_histogram_values - result["total_seconds_offset"]
else:
np_histogram_values = values
low, high = _get_range(values)
inlier_mask = (low <= values) & (values <= high)
n_low_outliers = (values < low).sum()
n_high_outliers = (high < values).sum()
result = {"n_low_outliers": n_low_outliers, "n_high_outliers": n_high_outliers}
result.update(n_low_outliers=n_low_outliers, n_high_outliers=n_high_outliers)
np_histogram_inliers = np_histogram_values[inlier_mask]
result["bin_counts"], result["bin_edges"] = np.histogram(
np_histogram_values[inlier_mask]
np_histogram_inliers, range=_get_safe_hist_range(np_histogram_inliers)
)
if ax is None:
return result
n, bins, patches = ax.hist(values[inlier_mask])
histogram_inliers = values[inlier_mask]
n, bins, patches = ax.hist(
histogram_inliers, range=_get_safe_hist_range(histogram_inliers)
)
n_out = n_low_outliers + n_high_outliers
if not n_out:
return result
Expand Down
21 changes: 21 additions & 0 deletions skrub/_reporting/tests/test_plotting.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,24 @@ def test_histogram():
data = pd.Series([0.0])
_, hist = _plotting.histogram(data)
assert (hist["n_low_outliers"], hist["n_high_outliers"]) == (0, 0)

# Test a column with values very close to each other.
# There are less than 10 (default number of histogram bins) representable
# floating-point numbers in the range, which would cause numpy or
# matplotlib histogram to raise an exception without appropriate handling
# in skrub.
low = np.float32(10.0)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a comment here explaining what this line is for?

high = np.nextafter(low, 11.0)

data = pd.Series([low, high])
_, hist = _plotting.histogram(data)
assert hist["n_low_outliers"] == 0
assert hist["n_high_outliers"] == 0

# all infinite. +- inf are outliers
data = pd.Series(
[None, None, float("nan"), float("-inf"), float("-inf"), float("inf")]
)
_, hist = _plotting.histogram(data)
assert hist["n_low_outliers"] == 2
assert hist["n_high_outliers"] == 1
Loading