Skip to content

Commit a4e1372

Browse files
committed
fix: solve minor issues in the histogram selection. refactoring done
1 parent 37632cc commit a4e1372

5 files changed

Lines changed: 74 additions & 52 deletions

File tree

docs/conf.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import re
1010
import sys
1111
from pathlib import Path
12+
import tomllib
1213

1314
DOCS_DIR = Path(__file__).resolve().parent
1415
ROOT_DIR = DOCS_DIR.parent
@@ -18,15 +19,15 @@
1819

1920

2021
def _read_release() -> str:
21-
init_file = ROOT_DIR / "src" / "khisto" / "__init__.py"
22-
match = re.search(
23-
r'^__version__\s*=\s*"(?P<version>[^"]+)"',
24-
init_file.read_text(encoding="utf-8"),
25-
re.MULTILINE,
26-
)
27-
if match is None:
28-
raise RuntimeError(f"Could not determine khisto version from {init_file}")
29-
return match.group("version")
22+
pyproject_file = ROOT_DIR / "pyproject.toml"
23+
data = tomllib.loads(pyproject_file.read_text(encoding="utf-8"))
24+
25+
try:
26+
return data["project"]["version"]
27+
except KeyError as exc:
28+
raise RuntimeError(
29+
f"Could not determine khisto version from {pyproject_file}"
30+
) from exc
3031

3132
project = 'khisto-python'
3233
copyright = '2026, The Khiops Team'

docs/index.rst

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ Get started
3232

3333
.. code-block:: bash
3434
35-
pip install khisto # core (NumPy only)
35+
pip install khisto # core (NumPy only)
3636
pip install "khisto[matplotlib]" # + plotting
3737
3838
.. code-block:: python
@@ -100,4 +100,3 @@ Get started
100100

101101
API Comparison <api_comparison>
102102
Demo <demo>
103-

src/khisto/array/histogram/api.py

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,20 +34,19 @@ def _select_histogram(
3434
"""
3535
if max_bins is not None:
3636
# Find the finest granularity that respects max_bins
37-
selected = None
38-
for r in histogram_results:
37+
for r in reversed(histogram_results):
3938
if len(r) <= max_bins:
40-
selected = r
41-
else:
42-
break
39+
return r
4340
# If no histogram respects the constraint, use the coarsest one
44-
return selected if selected is not None else histogram_results[0]
41+
return histogram_results[0]
4542
else:
46-
# Return the best (optimal) histogram
43+
# Return the best histogram (optimal in terms of interpretability)
44+
# There is only one best histogram, so we return the first one we find
4745
for r in reversed(histogram_results):
4846
if r.is_best:
4947
return r
5048
# Fallback to finest granularity if no best is marked
49+
# It is assumed to be the best because it is the finest granularity
5150
return histogram_results[-1]
5251

5352

src/khisto/core/backend.py

Lines changed: 55 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,16 @@
1111
import subprocess
1212
import tempfile
1313
from dataclasses import dataclass, field
14-
from typing import Any, Optional
14+
from pathlib import Path
15+
from typing import TYPE_CHECKING, Any
1516

1617
import numpy as np
17-
from numpy.typing import NDArray
1818

1919
from khisto import KHISTO_BIN_DIR, logger
2020

21+
if TYPE_CHECKING:
22+
from numpy.typing import NDArray
23+
2124

2225
@dataclass
2326
class _HistogramPayload:
@@ -68,18 +71,18 @@ class _KhistoOutput:
6871

6972
tool: str = ""
7073
version: str = ""
71-
bestHistogram: Optional[_HistogramPayload] = None
72-
histogramSeries: Optional[_SeriesPayload] = None
74+
bestHistogram: _HistogramPayload = field(default_factory=_HistogramPayload)
75+
histogramSeries: _SeriesPayload = field(default_factory=_SeriesPayload)
7376

7477
@classmethod
7578
def from_dict(cls, data: dict[str, Any]) -> _KhistoOutput:
76-
best = data.get("bestHistogram")
77-
series = data.get("histogramSeries")
79+
if "bestHistogram" not in data or "histogramSeries" not in data:
80+
raise ValueError("Missing required fields: bestHistogram, histogramSeries")
7881
return cls(
7982
tool=data.get("tool", ""),
8083
version=data.get("version", ""),
81-
bestHistogram=_HistogramPayload.from_dict(best) if best else None,
82-
histogramSeries=_SeriesPayload.from_dict(series) if series else None,
84+
bestHistogram=_HistogramPayload.from_dict(data["bestHistogram"]),
85+
histogramSeries=_SeriesPayload.from_dict(data["histogramSeries"]),
8386
)
8487

8588

@@ -128,9 +131,42 @@ class HistogramResult:
128131
spike_interval_number: int = 0
129132
empty_interval_number: int = 0
130133

134+
def __post_init__(self) -> None:
135+
"""Validate the histogram data."""
136+
137+
# Verify all arrays have the same length
138+
n_bins = {
139+
len(self.lower_bounds),
140+
len(self.upper_bounds),
141+
len(self.frequencies),
142+
len(self.probabilities),
143+
len(self.densities),
144+
}
145+
if len(n_bins) != 1:
146+
raise ValueError("all array sizes must be equal")
147+
elif len(self.lower_bounds) < 1:
148+
raise ValueError("all arrays must have at least one element")
149+
150+
if not np.all(self.lower_bounds <= self.upper_bounds):
151+
raise ValueError("lower_bounds must be less than upper_bounds")
152+
if not np.all(self.frequencies >= 0):
153+
raise ValueError("frequencies must be non-negative")
154+
if not np.all(self.probabilities >= 0):
155+
raise ValueError("probabilities must be non-negative")
156+
if not np.all(self.densities >= 0):
157+
raise ValueError("densities must be non-negative")
158+
159+
# Verify lower_bounds are equal to upper_bounds for adjacent bins
160+
print(self.lower_bounds, self.upper_bounds)
161+
if not np.all(self.lower_bounds[1:] == self.upper_bounds[:-1]):
162+
raise ValueError(
163+
"lower_bounds must be equal to upper_bounds for adjacent bins"
164+
)
165+
131166
@property
132167
def bin_edges(self) -> NDArray[np.float64]:
133-
"""Return bin edges array (n_bins + 1 values)."""
168+
"""Return bin edges array (n_bins + 1 values).
169+
lower_bounds and upper_bounds are equal for adjacent bins."""
134170
return np.concatenate([self.lower_bounds, [self.upper_bounds[-1]]])
135171

136172
@property
@@ -148,18 +184,6 @@ def __len__(self) -> int:
148184
return len(self.lower_bounds)
149185

150186

151-
def _to_result(h: _HistogramPayload, **kwargs: Any) -> HistogramResult:
152-
"""Convert a JSON histogram payload to a HistogramResult."""
153-
return HistogramResult(
154-
lower_bounds=np.asarray(h.lowerBounds, dtype=np.float64),
155-
upper_bounds=np.asarray(h.upperBounds, dtype=np.float64),
156-
frequencies=np.asarray(h.frequencies, dtype=np.int64),
157-
probabilities=np.asarray(h.probabilities, dtype=np.float64),
158-
densities=np.asarray(h.densities, dtype=np.float64),
159-
**kwargs,
160-
)
161-
162-
163187
def _format_runtime_error(
164188
summary: str, cmd: list[str], details: str | None = None
165189
) -> str:
@@ -170,21 +194,21 @@ def _format_runtime_error(
170194
return message
171195

172196

173-
def _process_histogram_file(
174-
file_path: str,
175-
) -> list[HistogramResult]:
197+
def _process_histogram_file(file_path: Path) -> list[HistogramResult]:
176198
"""Process exploratory JSON generated by khisto CLI."""
177-
with open(file_path, "r") as temp_output_file:
178-
khisto_output: _KhistoOutput = _KhistoOutput.from_dict(
179-
json.load(temp_output_file)
180-
)
199+
with open(file_path, "r") as file:
200+
khisto_output: _KhistoOutput = _KhistoOutput.from_dict(json.load(file))
181201

182202
histogram_series = khisto_output.histogramSeries
183203
best_idx = histogram_series.interpretableHistogramNumber - 1
184204

185205
return [
186-
_to_result(
187-
h,
206+
HistogramResult(
207+
lower_bounds=np.asarray(h.lowerBounds, dtype=np.float64),
208+
upper_bounds=np.asarray(h.upperBounds, dtype=np.float64),
209+
frequencies=np.asarray(h.frequencies, dtype=np.int64),
210+
probabilities=np.asarray(h.probabilities, dtype=np.float64),
211+
densities=np.asarray(h.densities, dtype=np.float64),
188212
is_best=(i == best_idx),
189213
granularity=histogram_series.granularities[i],
190214
level=histogram_series.levels[i],
@@ -268,7 +292,7 @@ def compute_histograms(x: np.ndarray) -> list[HistogramResult]:
268292
raise RuntimeError(message) from e
269293

270294
try:
271-
return _process_histogram_file(temp_output_file.name)
295+
return _process_histogram_file(Path(temp_output_file.name))
272296
except json.JSONDecodeError as e:
273297
message = _format_runtime_error(
274298
"khisto produced invalid JSON output",

src/khisto/matplotlib/hist.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,8 @@
88

99
from typing import TYPE_CHECKING, Any, Literal, Optional
1010

11-
import numpy as np
12-
1311
import matplotlib.pyplot as plt
12+
import numpy as np
1413
from matplotlib.axes import Axes
1514

1615
from khisto.array import histogram as khisto_histogram
@@ -36,7 +35,7 @@ def _apply_cumulative(
3635
hist_values: NDArray[np.float64],
3736
bin_edges: NDArray[np.float64],
3837
*,
39-
density: bool,
38+
density: bool = False,
4039
reverse: bool = False,
4140
) -> NDArray[np.float64]:
4241
"""Accumulate histogram values using matplotlib-compatible semantics."""

0 commit comments

Comments
 (0)