From 2902ceceb9257eaeb7925ea12c25b51d4fd3bd84 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Mon, 16 Dec 2024 11:37:09 -0300 Subject: [PATCH] Limit the number of bins for histograms to avoid crashes. --- crates/ark/src/data_explorer/histogram.rs | 25 +++++++++++++++++++ .../src/modules/positron/r_data_explorer.R | 5 ++++ 2 files changed, 30 insertions(+) diff --git a/crates/ark/src/data_explorer/histogram.rs b/crates/ark/src/data_explorer/histogram.rs index 4c3d1d062..1a3afb323 100644 --- a/crates/ark/src/data_explorer/histogram.rs +++ b/crates/ark/src/data_explorer/histogram.rs @@ -632,4 +632,29 @@ mod tests { ); }) } + + #[test] + fn test_limit_bins() { + // Regression test for https://github.com/posit-dev/positron/issues/5744 + r_task(|| { + let column = harp::parse_eval_global("rep(c(1:10, 5e7), 10)").unwrap(); + + let hist = profile_histogram( + column.sexp, + &ColumnHistogramParams { + method: ColumnHistogramParamsMethod::FreedmanDiaconis, + // If num_bins wasn't set to 10, that would generate almost 200k bins + num_bins: 10, + quantiles: None, + }, + &default_options(), + ) + .unwrap(); + + assert_match!(hist, ColumnHistogram { bin_edges, bin_counts, .. } => { + assert_eq!(bin_edges.len(), 11); + assert_eq!(bin_counts.len(), 10); + }); + }) + } } diff --git a/crates/ark/src/modules/positron/r_data_explorer.R b/crates/ark/src/modules/positron/r_data_explorer.R index 40647c86b..6a95fa8b1 100644 --- a/crates/ark/src/modules/positron/r_data_explorer.R +++ b/crates/ark/src/modules/positron/r_data_explorer.R @@ -519,5 +519,10 @@ histogram_num_bins <- function(x, method, fixed_num_bins) { } } + # Honor the maximum amount of bins requested by the front-end. + if (!is.null(fixed_num_bins) && num_bins > fixed_num_bins) { + num_bins <- fixed_num_bins + } + as.integer(num_bins) }