cap the default number of bins at 200 #421
Conversation
src/transforms/bin.js
Outdated
| if (thresholds === undefined) { | ||
| return (...args) => Math.min(200, thresholdScott(...args)); | ||
| } | ||
| if (thresholds === undefined) return thresholdDefault; |
There was a problem hiding this comment.
Can now be a default argument again.
There was a problem hiding this comment.
Related to the facet option #450, if we have a default implementation strategy for an option, there should be a way to specify it explicitly rather than requiring you to pass undefined. So, let’s call the min(200, scott) strategy auto, so that you can specify it explicitly?
I was separately wondering if the max of 200 should apply to any of the other threshold strategies, and if so, should it be a separate option, say thresholdsMax which defaults to 200? Or are we doing this only because Scott’s normal reference rule is prone to too many bins? It’s nice, though not obvious, that using scott instead of auto will disable the cap.
Lastly, I was wondering if the 200 should be dependent on the dimensions of the chart. That might suggest having a separate thresholdsMax option, too, which perhaps could somehow default to a value derived from the chart dimensions. Though these dimensions are not currently made available to transforms so it’d require further plumbing to make a smart default, and in any case, I think 200 is almost certainly fine for now.
|
Rebased and made "auto" explicit. (I don't think we should make this value of 200 configurable or magical; we just want to avoid the dramatic explosion of Scott’s rule when the distribution is highly concentrated around an extremum.) |
(see #417)
(200 is working for almost any chart size; 100 would be a fine choice too)