Skip to content

Commit

Permalink
Limit the number of bins for histograms to avoid crashes. (#655)
Browse files Browse the repository at this point in the history
  • Loading branch information
dfalbel authored Dec 17, 2024
1 parent 7a0acf9 commit e0b1620
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
25 changes: 25 additions & 0 deletions crates/ark/src/data_explorer/histogram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
})
}
}
5 changes: 5 additions & 0 deletions crates/ark/src/modules/positron/r_data_explorer.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

0 comments on commit e0b1620

Please sign in to comment.