-
Notifications
You must be signed in to change notification settings - Fork 187
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
polylinear (aka piecewise linear) scales with an unspecified range #530
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.
👍
test/plots/polylinear.js
Outdated
{ date: new Date(2013, 3, 5, 13, 0), text: "Initiate" }, | ||
{ date: new Date(2013, 3, 11, 13, 0), text: "Begin" }, | ||
{ date: new Date(2013, 3, 13, 20, 0), text: "Entry" }, | ||
{ date: new Date(2013, 3, 15, 0, 0), text: "Test" }, | ||
{ date: new Date(2013, 3, 16, 0, 0), text: "Drive" }, | ||
{ date: new Date(2013, 3, 17, 8, 0), text: "Drive" }, | ||
{ date: new Date(2013, 3, 18, 15, 0), text: "Brake" }, | ||
{ date: new Date(2013, 3, 20, 10, 0), text: "Stop" }, | ||
{ date: new Date(2013, 3, 23, 14, 0), text: "Shutdown" } |
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.
These are local times; could we use UTC to avoid being dependent on the local timezone? (And then use d3.utcDays instead of d3.timeDays etc. below.)
I’d also prefer if you changed your prettier settings to avoid the spaces inside the curly braces. 🙂
obsolete remark This PR is not ready because it doesn't work for color schemes. The problem is upstream, see https://observablehq.com/d/0659c57ac79f0bbc I have a patch that does this inside Plot, but we should probably fix d3-scale instead (though it might be quite a bit more complicated!). |
I've added a fix for polylinear color schemes. (It seems more difficult to fix in d3-scale where .domain() can be called several times and the original interpolate function would be lost.) |
(seems like the most straightforward approach)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
(In practice, one probably wouldn’t try to use both options together. Still, looks safer.)
I've verified that it's compatible with #484. |
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 has changed quite a bit since I last reviewed, so retracting my approval until I have a chance to review again.
Actually, it looks like you folded this work into #538, so we should probably close this PR. (Though in the future I think it would still be my preference to keep them separate.) |
Now merged in #538
A polylinear scale (or piecewise linear) scale is defined by example as:
In Plot, specifying the domain and range works as expected, but we must also specify
type: "linear"
, otherwise Plot decides (rightly) that it might be an ordinal scale (a point scale).However if we specify only the domain, and let the range be determined by the dimensions of the Plot (say, for the y axis, by autoScaleRangeY), only the two first elements of the domain are mapped to [ymin, ymax], and the resulting chart is broken.
This pull-request closes that gap.
The (made-up) example could be adapted to a real-life scenario of a mission which has long periods of idle time and shorter periods of more intense activity.