refactor: rename variables for clarity and improve error handling in histogram processing

Author: ElouenGinat

src/khisto/array/histogram/api.py

@@ -15,7 +15,7 @@
 
 
 def _select_histogram(
-    results: list[HistogramResult],
+    histogram_results: list[HistogramResult],
     max_bins: Optional[int] = None,
 ) -> HistogramResult:
     """Select the appropriate histogram from the list of results.
@@ -35,38 +35,20 @@ def _select_histogram(
     if max_bins is not None:
         # Find the finest granularity that respects max_bins
         selected = None
-        for r in results:
+        for r in histogram_results:
             if len(r) <= max_bins:
                 selected = r
             else:
                 break
         # If no histogram respects the constraint, use the coarsest one
-        return selected if selected is not None else results[0]
+        return selected if selected is not None else histogram_results[0]
     else:
         # Return the best (optimal) histogram
-        for r in results:
+        for r in reversed(histogram_results):
             if r.is_best:
                 return r
         # Fallback to finest granularity if no best is marked
-        return results[-1]
-
-
-def _apply_cumulative(
-    hist_values: NDArray[np.float64],
-    bin_edges: NDArray[np.float64],
-    *,
-    density: bool,
-    reverse: bool = False,
-) -> NDArray[np.float64]:
-    """Accumulate histogram values using matplotlib-compatible semantics."""
-    if density:
-        source_values = hist_values * np.diff(bin_edges)
-    else:
-        source_values = hist_values
-
-    if reverse:
-        return np.cumsum(source_values[::-1])[::-1]
-    return np.cumsum(source_values)
+        return histogram_results[-1]
 
 
 def histogram(
@@ -125,15 +107,20 @@ def histogram(
     """
     arr = np.asarray(a, dtype=np.float64).flatten()
 
+    if max_bins is not None and max_bins <= 0:
+        raise ValueError("max_bins must be a positive integer or None.")
+
     # Filter values by range if specified
     if range is not None:
         min_val, max_val = range
         arr = arr[(arr >= min_val) & (arr <= max_val)]
 
-    results = compute_histograms(arr)
-    result = _select_histogram(results, max_bins=max_bins)
+    histogram_results = compute_histograms(arr)
+    histogram_result = _select_histogram(histogram_results, max_bins=max_bins)
 
     if density:
-        return result.density.copy(), result.bin_edges.copy()
+        return histogram_result.density.copy(), histogram_result.bin_edges.copy()
     else:
-        return result.frequency.astype(np.float64), result.bin_edges.copy()
+        return histogram_result.frequency.astype(
+            np.float64
+        ), histogram_result.bin_edges.copy()

src/khisto/core/backend.py

@@ -167,54 +167,43 @@ def _parse_file_type(file_path: pathlib.Path) -> None:
         raise ValueError(f"Unrecognized histogram file name: {file_path.name}")
 
 
-def _best_index(
-    series: _SeriesPayload, best: Optional[_HistogramPayload]
-) -> Optional[int]:
-    """Determine the best histogram index from the series."""
-    if series.interpretableHistogramNumber > 0:
-        return series.interpretableHistogramNumber - 1
-    if best is not None:
-        for i, h in enumerate(series.histograms):
-            if h.lowerBounds == best.lowerBounds:
-                return i
-    return None
-
-
 def _process_histogram_files(temp_dir: str, base_name: str) -> list[HistogramResult]:
     """Process exploratory JSON generated by khisto CLI."""
     output_file = pathlib.Path(temp_dir) / f"{base_name}.json"
     _parse_file_type(output_file)
 
     with output_file.open("r", encoding="utf-8") as f:
-        output: _KhistoOutput = _KhistoOutput.from_dict(json.load(f))
-
-    if not output.histogramSeries or not output.histogramSeries.histograms:
-        if output.bestHistogram is None:
-            raise ValueError("No histogram data found")
-        return [_to_result(output.bestHistogram, is_best=True)]
+        khisto_output: _KhistoOutput = _KhistoOutput.from_dict(json.load(f))
 
-    series = output.histogramSeries
-    n = len(series.histograms)
-    best_idx = _best_index(series, output.bestHistogram)
+    histogram_series = khisto_output.histogramSeries
+    n = len(histogram_series.histograms)
+    best_idx = histogram_series.interpretableHistogramNumber - 1
     granularities = (
-        series.granularities if len(series.granularities) == n else list(range(n))
+        histogram_series.granularities
+        if len(histogram_series.granularities) == n
+        else list(range(n))
     )
 
-    def _get(lst: list, i: int, default=0):
-        return lst[i] if i < len(lst) else default
+    def _get(lst: list, i: int):
+        if i < len(lst):
+            return lst[i]
+        else:
+            raise ValueError(
+                f"Expected at least {i + 1} values in list, got {len(lst)}"
+            )
 
     return [
         _to_result(
             h,
             is_best=(i == best_idx),
             granularity=granularities[i],
-            level=_get(series.levels, i, 0.0),
-            information_rate=_get(series.informationRates, i, 0.0),
-            peak_interval_number=_get(series.peakIntervalNumbers, i),
-            spike_interval_number=_get(series.spikeIntervalNumbers, i),
-            empty_interval_number=_get(series.emptyIntervalNumbers, i),
+            level=_get(histogram_series.levels, i),
+            information_rate=_get(histogram_series.informationRates, i),
+            peak_interval_number=_get(histogram_series.peakIntervalNumbers, i),
+            spike_interval_number=_get(histogram_series.spikeIntervalNumbers, i),
+            empty_interval_number=_get(histogram_series.emptyIntervalNumbers, i),
         )
-        for i, h in enumerate(series.histograms)
+        for i, h in enumerate(histogram_series.histograms)
     ]