Skip to content

Auto mark: no display with non-zero reducer and bar mark #1340

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

Closed
Tracked by #1252
tophtucker opened this issue Mar 15, 2023 · 3 comments · Fixed by #1368
Closed
Tracked by #1252

Auto mark: no display with non-zero reducer and bar mark #1340

tophtucker opened this issue Mar 15, 2023 · 3 comments · Fixed by #1368
Assignees
Labels
bug Something isn’t working

Comments

@tophtucker
Copy link
Contributor

tophtucker commented Mar 15, 2023

See examples. You get a bunch of zero-height bars:

Plot.auto(olympians, {x: "date_of_birth", y: { value: "height", reduce: "mean" }, mark: "bar"}).plot()

The easy fix is to change the last row of this to use rectY instead of rect, on the theory that Plot.auto never has enough information to use Plot.rect effectively:

plot/src/marks/auto.js

Lines 159 to 174 in c489290

case "bar":
mark = yZero
? isOrdinalReduced(xReduce, X)
? barY
: rectY
: xZero
? isOrdinalReduced(yReduce, Y)
? barX
: rectX
: isOrdinalReduced(xReduce, X) && isOrdinalReduced(yReduce, Y)
? cell
: isOrdinalReduced(xReduce, X)
? barY
: isOrdinalReduced(yReduce, Y)
? barX
: rect;

That works in this case:

But it breaks the heatmap, because it gives it a zero baseline:

Plot.auto(penguins, {x: "culmen_length_mm", y: "body_mass_g", color: "count"}).plot()

Because we're inferring whether to show the zero baseline from the mark type:

plot/src/marks/auto.js

Lines 217 to 220 in c489290

// If zero-ness is not specified, default based on whether the resolved mark
// type will include a zero baseline. TODO Move this to autoSpec.
if (xZero === undefined) xZero = transform !== binX && (mark === barX || mark === areaX || mark === rectX);
if (yZero === undefined) yZero = transform !== binY && (mark === barY || mark === areaY || mark === rectY);

We could change that bit. But, moreover, the heatmap example shows that Plot.auto does sometimes have enough information to specify a rect mark: in the case of 2D binning. So just switching rect to rectY feels wrong.

@tophtucker tophtucker changed the title Auto: bar override fails for non-additive reducers Auto mark: no display with non-zero reducer and bar mark Mar 15, 2023
@mbostock mbostock added the bug Something isn’t working label Mar 15, 2023
@mbostock
Copy link
Member

The mark-based zero check is also forgetting about rule. This should have a y = 0 rule like rect does.

untitled (25)

Plot.auto(olympians, {x: "date_of_birth", y: {value: "height", reduce: "mean"}, mark: "rule"}).plot()

@tophtucker
Copy link
Contributor Author

@mbostock Can you think of a test case where using rect instead of rectY would break aside from the zero-baseline issue? These look identical:

Plot.rect(penguins, Plot.bin({fill: "count"}, {x: "culmen_length_mm", y: "body_mass_g"})).plot()
Plot.rectY(penguins, Plot.bin({fill: "count"}, {x: "culmen_length_mm", y: "body_mass_g"})).plot()

@mbostock
Copy link
Member

The only difference between rect and rectY is that rectY gives you an implicit stackY transform and defaults y to the identity transform. So, if you’re supply y1 and y2 anyway, which the bin transform will do, there is no difference between rect and rectY. So I think it will be acceptable to use rectY here.

Regarding the zero baseline issue, maybe the answer is to address the TODO to move the zero-ness determination into autoSpec, rather than splitting the defaults into two places. You’ll have to rewrite the logic to be based more on “first principles” than the concrete mark type, but that might end up being a cleaner expression of the desired logic besides.

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.

2 participants