Skip to content
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

Closed
wants to merge 12 commits into from

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Sep 3, 2021

Now merged in #538

A polylinear scale (or piecewise linear) scale is defined by example as:

d3.scaleLinear()
  .domain([0, 100, 200, 300, 400])
  .range(["red", "blue", "green", "lime", "yellow"]) // or pixels for a position scale

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.

Capture d’écran 2021-09-03 à 16 55 51

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.

const times = [
  new Date(2013, 3, 5),
  new Date(2013, 3, 11),
  new Date(2013, 3, 14),
  new Date(2013, 3, 16),
  new Date(2013, 3, 18),
  new Date(2013, 3, 21),
  new Date(2013, 3, 27)
];

const events = [
  { 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" }
];

export default async function() {
  return Plot.plot({
    grid: true,
    x: {
      domain: times,
      type: "linear",
      ticks: [...d3.timeDays(...d3.extent(times)), times[times.length-1]],
      tickFormat: d3.timeFormat("%d"),
      inset: 20,
      label: "date →"
    },
    color: { scheme: "cool" },
    marks: [
      Plot.barX(d3.pairs(times), {x1: "0", x2: "1", fill: (_,i) => i}),
      Plot.dotX(events, {x: "date", fill: "white"}),
      Plot.textX(events, {x: "date", text: "text", dx: -5, dy: -10, fill: "white", textAnchor: "start"})
    ],
    height: 90
  });
}

@Fil Fil added the bug Something isn’t working label Sep 3, 2021
@Fil Fil requested a review from zanarmstrong September 3, 2021 14:57
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines 15 to 23
{ 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" }
Copy link
Member

@mbostock mbostock Sep 7, 2021

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. 🙂

@Fil
Copy link
Contributor Author

Fil commented Sep 7, 2021

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

Capture d’écran 2021-09-07 à 20 02 35

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!).

@Fil
Copy link
Contributor Author

Fil commented Sep 7, 2021

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)
@Fil

This comment has been minimized.

@Fil Fil marked this pull request as ready for review September 8, 2021 16:29
@Fil Fil marked this pull request as draft September 8, 2021 16:32
@Fil

This comment has been minimized.

@Fil Fil marked this pull request as ready for review September 8, 2021 16:52
@Fil Fil mentioned this pull request Sep 9, 2021
7 tasks
@Fil
Copy link
Contributor Author

Fil commented Sep 10, 2021

I've verified that it's compatible with #484.

Fil added a commit that referenced this pull request Sep 15, 2021
Copy link
Member

@mbostock mbostock left a 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.

@mbostock
Copy link
Member

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.)

@Fil Fil closed this Sep 24, 2021
@Fil Fil deleted the fil/piecewise-linear branch September 24, 2021 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants