-
Notifications
You must be signed in to change notification settings - Fork 185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ensure thresholds consistency across facets #1226
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this.
Did you consider switching to contour.contour directly since we are now effectively opting-out of d3-contour’s built-in ticks logic?
src/marks/contour.js
Outdated
@@ -158,7 +158,12 @@ function contourGeometry({thresholds, interval, ...options}) { | |||
? thresholds.range(...(([min, max]) => [thresholds.floor(min), max])(finiteExtent(VV))) | |||
: typeof thresholds === "function" | |||
? thresholds(V, ...finiteExtent(VV)) | |||
: thresholds; | |||
: typeof thresholds === "number" | |||
? ticks(...nice(...finiteExtent(VV), thresholds), thresholds) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn’t sufficient. We’ll need these lines too (d3/d3-contour#68):
Co-authored-by: Mike Bostock <[email protected]>
b76ee01
to
67b6e4f
Compare
* ensure thresholds consistency across facets * arrayify Co-authored-by: Mike Bostock <[email protected]> * nice ticks; use contour * polish Co-authored-by: Mike Bostock <[email protected]>
Using thresholds: 10 would create 10 different levels on each facet; this ensures they are consistent, like we do with the bin transform.