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.

Sign up for GitHub

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

Jump to bottom

Radial chart radius field unexpectedly changes arc stack behavior #7957

Open
5 tasks done
anbnyc opened this issue Feb 2, 2022 · 7 comments · May be fixed by #9512
Open
5 tasks done

Radial chart radius field unexpectedly changes arc stack behavior #7957

anbnyc opened this issue Feb 2, 2022 · 7 comments · May be fixed by #9512
Assignees
@kanitw
Labels
Bug 🐛 P2 Important Issues that should be fixed soon

Comments

@anbnyc
Copy link

anbnyc commented Feb 2, 2022

Please:

  • Check for duplicate issues. Please file separate requests as separate issues.
  • Describe how to reproduce the bug.
  • Use the latest versions of Vega and Vega-Lite.
  • Provide an minimal, reproducible example spec in JSON, wrapped by triple backticks like this:

Using field for radius other than the one provided to theta unexpectedly breaks the stack behavior for theta. It seems like radius should be able use a different scale than theta. For example, with the following code, the arcs do not stack on the theta variable as expected.

{
  "$schema": "https://vega.github.io/schema/vega-lite/v5.json",
  "data": {
    "values": [
      { "segment": 1, "value": 8 },
      { "segment": 2, "value": 7 },
      { "segment": 3, "value": 6 }
    ]
  },
  "mark": {"type": "arc", "innerRadius": 20, "stroke": "#fff", "opacity": 0.5},
  "encoding": {
    "theta": {"field": "segment", "type": "quantitative", "stack": true},
    "radius": {"field": "value", "scale": {"type": "sqrt", "zero": true, "rangeMin": 20}}
  }
}
@anbnyc anbnyc added the Bug 🐛 label Feb 2, 2022
@HelenaV
Copy link

HelenaV commented Oct 31, 2022

Just ran in this bug. Ah, I really need this radial chart now. Any updates here?

@qnzhou
Copy link

qnzhou commented Sep 29, 2023

I am able to reproduce this bug: vega/altair#3204

This bug is making Altair and Vega-Lite incapable to creating radial chart. Is there any plan of fixing it?

@PBI-David
Copy link
Contributor

Same bug reported on SO: https://stackoverflow.com/questions/77269555/how-do-you-create-a-radial-plot-with-pre-defined-radius-data/77270813#77270813

@domoritz domoritz added the P2 Important Issues that should be fixed soon label Oct 11, 2023
@hydrosquall
Copy link
Member

I was wondering if doing this would be possible with plain Vega, and noticed the other examples are using a uniform theta.

If we can achieve the desired graph with plain Vega, then this should stay here. If the desired graph isn't producible with plain Vega, we should relocate this issue (using the compiled spec) to the Vega repo

References:

https://groups.google.com/g/vega-js/c/oaT2fq1dkYs

vega/vega#2023

https://observablehq.com/@stanisouce/vega-wind-rose-or-radial-stacked-bar-chart-in-progress

@hydrosquall
Copy link
Member

hydrosquall commented Jan 20, 2025 •
edited
Loading

I have a fix to repair the "Broken Version" by modifying the compiled Vega ( ex ) . This seems correct since the arc with the smallest arc length also has the longest radius, and vice versa, just like in the source data.

Before - Compiled from Vega-Lite After - Hand-Edited Vega
Image Image

Reading @qnzhou 's investigation from Altair into the stack transform was helpful! I found 1 change was needed to the compiled spec to enable the repair:

  1. The groupBy in the stack transform should be an empty array instead of value
Image

I see support for arc was added in #6201, and the last big change to that encoding was in #7098, so I might have a look there to improve my understanding of how we might apply this change programmatically. Open to other approaches too ( cc @kanitw )

Archive

Outdated comment on what changed
  1. spec.marks[0].encode.update.outerRadius.field should be value instead of segment
  2. spec.scales[1].domain.fieldshould also bevalueinstead ofsegment`

@hydrosquall hydrosquall linked a pull request Jan 20, 2025 that will close this issue
Open
@hydrosquall
Copy link
Member

hydrosquall commented Jan 20, 2025 •
edited
Loading

I have a starter for a fix that improves the radial case here .

I would like to avoid making this check hyper-specific to polar / no color encoding case if possible as it seems like a check that may be too-specific, but I've opened the PR anyways to get the discussion started.

@hydrosquall
Copy link
Member

hydrosquall commented Jan 25, 2025 •
edited
Loading

I dug into this further because while the groupBy field made the original bug report case better, it broke existing test cases. You can see an example on the updated snapshots in #9512

The only difference was that the existing vega-lite code handles stacked theta well for ordinal scale ( see this link, but the unexpected behaviors happen with the quantatitative scale in this link .

@kanitw , do you think this is something that we would end up changing in the scale or the stack.ts sections of the VL code

Writing it out helped - omitting the groupBy when we use a quantitative scale seems to work (fixes new bug and passes the existing tests) b19fc7f

@hydrosquall hydrosquall mentioned this issue Jan 26, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kanitw kanitw

Labels
Bug 🐛 P2 Important Issues that should be fixed soon
Projects
None yet
Milestone
No milestone
7 participants
@kanitw @domoritz @anbnyc @qnzhou @hydrosquall @HelenaV @PBI-David