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

The areaY mark’s interval option doesn’t work with y1 and y2 #1784

Closed
mbostock opened this issue Aug 2, 2023 · 0 comments · Fixed by #1828
Closed

The areaY mark’s interval option doesn’t work with y1 and y2 #1784

mbostock opened this issue Aug 2, 2023 · 0 comments · Fixed by #1828
Labels
bug Something isn’t working

Comments

@mbostock
Copy link
Member

mbostock commented Aug 2, 2023

The maybeDenseIntervalX transform used by areaY’s interval option uses the binX transform, but only outputs the y channel:

: bin({[k]: options?.reduce === undefined ? reduceFirst : options.reduce, filter: null}, options);

This means it doesn’t work if the areaY mark is using the y1 and y2 options:

untitled (49)

Plot.areaY(aapl.slice(0, 81), {
  x: "Date",
  y1: "Low",
  y2: "High",
  interval: "day",
  curve: "step"
}).plot()

It should instead be equivalent to this:

untitled (50)

Plot.areaY(
  aapl.slice(0, 81),
  Plot.binX(
    { y1: "first", y2: "first", filter: null },
    { x: "Date", y1: "Low", y2: "High", interval: "day", curve: "step" }
  )
).plot()

So somehow maybeDenseIntervalX will need to know to output y1 and y2 instead.

Ref. https://observablehq.com/d/39a059b4b6322d37

@mbostock mbostock added the bug Something isn’t working label Aug 2, 2023
mbostock added a commit that referenced this issue Aug 24, 2023
…1828)

* the denseInterval option for k must also reduce k1 and k2 if present

closes #1784

* Update src/transforms/bin.js

* conditional reduce of k, too

* prettier

---------

Co-authored-by: Mike Bostock <[email protected]>
chaichontat pushed a commit to chaichontat/plot that referenced this issue Jan 14, 2024
…bservablehq#1828)

* the denseInterval option for k must also reduce k1 and k2 if present

closes observablehq#1784

* Update src/transforms/bin.js

* conditional reduce of k, too

* prettier

---------

Co-authored-by: Mike Bostock <[email protected]>
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 a pull request may close this issue.

1 participant