vega-lite
vega-lite copied to clipboard
vega
duplicate parameter defintion error in layered charts
Original observed in this Altair issue comment: https://github.com/altair-viz/altair/pull/2528#issuecomment-992989438. Open the Chart in the Vega Editor
{
"params": [
{
"name": "highlight",
"select": {"type": "point", "clear": "mouseout", "on": "mouseover"}
}
],
"vconcat": [
{
"height":100,
"mark": {"type": "line", "point": true},
"encoding": {
"x": {"field": "date", "timeUnit": "year", "type": "nominal"},
"y": {"aggregate": "mean", "field": "price", "type": "quantitative"}
}
},
{
"mark": {"type": "rect"},
"encoding": {
"fill": {"aggregate": "mean", "field": "price", "type": "quantitative"},
"stroke": {
"condition": {"value": "black", "param": "highlight", "empty": false},
"value": null
},
"x": {"field": "date", "timeUnit": "year", "type": "temporal"}
}
}
],
"data": {"url": "data/stocks.csv"},
"$schema": "https://vega.github.io/schema/vega-lite/v5.2.0.json"
}
gives
When setting -upon first sight unrelated- "point": true to "point": false it will work:
Defining the parameter on the view, it will also work with "point": true.
{
"vconcat": [
{
"height": 100,
"mark": {"type": "line", "point": true},
"encoding": {
"x": {"field": "date", "timeUnit": "year", "type": "nominal"},
"y": {"aggregate": "mean", "field": "price", "type": "quantitative"}
},
"params": [
{
"name": "highlight",
"select": {"type": "point", "clear": "mouseout", "on": "mouseover"}
}
]
},
{
"mark": {"type": "rect"},
"encoding": {
"fill": {"aggregate": "mean", "field": "price", "type": "quantitative"},
"stroke": {
"condition": {"value": "black", "param": "highlight", "empty": false},
"value": null
},
"x": {"field": "date", "timeUnit": "year", "type": "temporal"}
}
}
],
"data": {"url": "data/stocks.csv"},
"$schema": "https://vega.github.io/schema/vega-lite/v5.2.0.json"
}
Raised here: https://github.com/altair-viz/altair/pull/2671#issuecomment-1244085790. Another example of this behaviour, Open the Chart in the Vega Editor
The following works and the PARAM_0 is applied to both VIEW_0 and VIEW_1 even though only "views": ["VIEW_0"] is defined within PARAM_0 :
{
"$schema": "https://vega.github.io/schema/vega-lite/v5.2.0.json",
"data": {"url": "data/cars.json"},
"params": [
{
"name": "PARAM_0",
"select": {
"type": "point",
"clear": "mouseout",
"encodings": ["x", "y"],
"nearest": true,
"on": "mouseover"
},
"views": ["VIEW_0"]
}
],
"layer": [
{
"name": "VIEW_0",
"mark": "circle",
"encoding": {
"color": {
"condition": {
"field": "Origin",
"type": "nominal",
"param": "PARAM_0",
"empty": false
},
"value": "lightgray"
},
"size": {
"condition": {"value": 250, "param": "PARAM_0", "empty": false},
"value": 20
},
"x": {"field": "Miles_per_Gallon", "type": "quantitative"},
"y": {"field": "Horsepower", "type": "quantitative"}
}
},
{
"name": "VIEW_1",
"mark": "text",
"encoding": {
"color": {
"condition": {"value": "black", "param": "PARAM_0", "empty": false},
"value": "transparent"
},
"text": {"field": "Origin", "type": "nominal"},
"x": {"field": "Miles_per_Gallon", "type": "quantitative"},
"y": {"field": "Horsepower", "type": "quantitative"}
}
}
]
}
But when changing the parameter to include both views ("views": ["VIEW_0", "VIEW_1"]), since both views contain references to the param PARAM_0 it gives the following error:
Duplicate signal name: "PARAM_0_tuple"
I found this hard to understand. Not really know how this can be easily improved as it is not possible to provide name definition on the layer.
This spec is another example: Open the Chart in the Vega Editor:
{
"data": {
"values": [
{"a": 1, "b": 1, "c": 1, "d": 1},
{"a": 2, "b": 2, "c": 1, "d": 2},
{"a": 3, "b": 1, "c": 2, "d": 3},
{"a": 4, "b": 2, "c": 2, "d": 4}
]
},
"facet": {"field": "c", "type": "nominal"},
"params": [
{
"name": "selected",
"select": {"type": "point", "fields": ["d"]},
"bind": "legend",
"views": ["view_1"]
}
],
"spec": {
"name": "view_1",
"mark": {"type": "line", "point": true},
"encoding": {
"color": {"field": "d", "type": "nominal"},
"x": {"field": "a", "type": "quantitative"},
"y": {"field": "b", "type": "quantitative"},
"opacity": {"condition": {"value": 1, "param": "selected"}, "value": 0.25}
}
},
"$schema": "https://vega.github.io/schema/vega-lite/v5.2.0.json"
}
When changing "point": true to "point": false the error disappear (but the chart won't appear as requested).
Manually defining both the circle and line mark will work though.
{
"data": {
"values": [
{"a": 1, "b": 1, "c": 1, "d": 1},
{"a": 2, "b": 2, "c": 1, "d": 2},
{"a": 3, "b": 1, "c": 2, "d": 3},
{"a": 4, "b": 2, "c": 2, "d": 4}
]
},
"facet": {"field": "c", "type": "nominal"},
"spec": {
"layer": [
{
"mark": "line",
"encoding": {
"color": {"field": "d", "type": "nominal"},
"opacity": {
"condition": {"value": 1, "param": "selected"},
"value": 0.25
},
"x": {"field": "a", "type": "quantitative"},
"y": {"field": "b", "type": "quantitative"}
},
"name": "view_1"
},
{
"mark": "circle",
"encoding": {
"color": {"field": "d", "type": "nominal"},
"opacity": {
"condition": {"value": 1, "param": "selected"},
"value": 0.25
},
"x": {"field": "a", "type": "quantitative"},
"y": {"field": "b", "type": "quantitative"}
}
}
]
},
"params": [
{
"name": "selected",
"select": {"type": "point", "fields": ["d"]},
"bind": "legend",
"views": ["view_1"]
}
],
"$schema": "https://vega.github.io/schema/vega-lite/v5.2.0.json"
}
Open the Chart in the Vega Editor
I assume the point=True will duplicate the spec under the hood into a layered chart, but it also duplicates the "name": "view_1" for both layers and that causes this error, since I define "name": "view_1" on only a single view in the manual layered chart.
Hi @mattijn, I was looking at a variant of your example https://github.com/vega/vega-lite/issues/7854#issuecomment-1315551841 above. Here is the version I was looking at: Open the Chart in the Vega Editor. My version is one step simpler than yours because mine has "point": false. (I also slightly changed the data so that there are lines present.)
If you open my version in the Vega editor and click on "Extended Vega-Lite Spec" at the bottom and then "Edit Extended Vega-Lite Spec", you'll see that the parameter has been moved from the top level to inside the spec object. Also warnings appear in the logs because of the presence of the "views" property in this non-top-level parameter.
I can't quite follow the code, but based on this comment, my guess is that it might be related to the normalizeGenericSpec function.
Does that give you any ideas?
That might be another issue.
I think the problem here is that the "point": true will duplicate the spec under the hood somewhere into a layered version of a line mark encoding and a circle mark encoding, where the latter is a duplicate of the mark line with only the the mark line to circle replaced. But since there is also a name definition ("name": "view_1"), it also duplicates that name and that is problematic since it is not allowed to have two identical name definitions in different chart objects in a layered chart.
If in the duplication process of creating circle mark chart object the key name is popped then the issue will resolve. I think.
If I modify your example into a manual layered version of a line and mark, Open the Chart in the Vega Editor, it starts with the same error as with "point": true (you can check and search for selected_tuple in the 'Compiled Vega'), but if you remove line 35 from the Vega-Lite specification (and thus removing the name of the second chart in the layered spec) than it works.
Can we find the location in the code where the "point": true is start being processed?
Thank you! I haven't completely processed what you wrote @mattijn, but I do think I tracked down where the "point": true starts to be processed. This is my best guess: https://github.com/vega/vega-lite/blob/2ddf38305215038b46ad4be22ac0372be130a5f8/src/normalize/pathoverlay.ts#L162-L174
Will changing this sentence (on line 170):
...pointOverlay
into
...omit(pointOverlay, ['name'])
be sufficient?
It seems that omit is a custom function defined in utils which delete keys from a copied dictionary where the delete command in typescript will not fail if the key is not detected.
Nope, that is one level too deep. I think my suggestion will turn this:
"name": "foo"
"mark": {"type":"point", "filled": true, "name": "bah"},
into:
"name": "foo"
"mark": {"type":"point", "filled": true},
Where we like to "omit" "name": "foo". Maybe somewhere around line 164?
Hi @mattijn, do you know what's the best way to test these kind of code changes? Is it opening a linked Vega Editor? Or is there a more natural choice like running the code in VS Code and using debugging? (All the (limited) JavaScript I've played with before has been website-focused, so I always tested it by using a web browser.)
There is a contributing page in the repo. It mentions VS Code as preferable IDE: https://github.com/vega/vega-lite/blob/next/CONTRIBUTING.md#suggested-programming-environment.
But I've no much experience in this either.
Thanks @mattijn. I'm just experimenting at this stage, but can you take a look at https://github.com/ChristopherDavisUCI/vega-lite/commit/1ced888950ce5580266fc1fc4f35a902238bd449 and see if you think it helps the error from https://github.com/vega/vega-lite/issues/7854#issuecomment-1315551841 ?
It does not seem to help your initial vconcat example. Edit. It doesn't fix your vconcat example at the top of this issue, but the Altair chart that originally produced that spec has already been fixed (thanks to the use of named views), so from the Altair side, I don't think we need to worry about your vconcat example. Here is the json produced by Altair for your example: Open the Chart in the Vega Editor
@ChristopherDavisUCI Nice catch, I missed the implementation of named views, this is great! For reference, produces
import altair as alt
from vega_datasets import data
stocks = data.stocks.url
highlight = alt.selection_point(name="highlight", on="mouseover", clear="mouseout")
line = (
alt.Chart(height=100)
.mark_line(point=True)
.encode(x=alt.X("date:N", timeUnit="year"), y=alt.Y("mean(price):Q"))
)
rect = (
alt.Chart()
.mark_rect()
.encode(
x=alt.X("date:T", timeUnit="year"),
fill=alt.Y("mean(price):Q"),
stroke=alt.condition(
highlight, alt.value("black"), alt.value(None), empty=False
),
)
.add_params(highlight)
)
alt.vconcat(line, rect, data=stocks)
{
"$schema": "https://vega.github.io/schema/vega-lite/v5.2.0.json",
"data": {
"url": "https://cdn.jsdelivr.net/npm/[email protected]/data/stocks.csv"
},
"params": [
{
"name": "highlight",
"select": {"clear": "mouseout", "on": "mouseover", "type": "point"},
"views": ["view_3"]
}
],
"vconcat": [
{
"encoding": {
"x": {"field": "date", "timeUnit": "year", "type": "nominal"},
"y": {"aggregate": "mean", "field": "price", "type": "quantitative"}
},
"height": 100,
"mark": {"point": true, "type": "line"}
},
{
"encoding": {
"fill": {"aggregate": "mean", "field": "price", "type": "quantitative"},
"stroke": {
"condition": {"empty": false, "param": "highlight", "value": "black"},
"value": null
},
"x": {"field": "date", "timeUnit": "year", "type": "temporal"}
},
"mark": "rect",
"name": "view_3"
}
]
}
which is valid. Only difference is the naming of "view_3" and that the parameter references it. Does this mean that this issue is no longer blocking the Altair 5 release or is there still a common pattern in Altair that is affected?
Thanks for checking this out, @binste! You're right that @mattijn's original chart now works in Altair, but I believe this is an example where an update is still needed:
import altair as alt
import pandas as pd
sel = alt.selection_point(fields=["c"])
df = pd.DataFrame({
"a": [1,2,3,4],
"b": [1,2,1,2],
"c": [1,1,2,2]
})
alt.Chart(df).mark_line(point=True).encode(
x="a",
y="b",
color="c:N",
).facet(
"c:N"
).add_params(sel)
Like in the other examples, changing to point=False will remove the problem.
I think my code here fixes this particular issue, but I'm not sure if it's a reasonable approach or if it causes other problems.
Yes there is still a common pattern affected. The second comment is still problematic. It was raised as well in https://github.com/altair-viz/altair/issues/2713.
I'm somehow surprised your code fix this issue @ChristopherDavisUCI. It is the right thing to do, but before the name key was not yet injected there, so it was done elsewhere. Maybe there is a check if there is a name defined it does not do this injection.
Does it pass the test bank of Vega-Lite? Otherwise turn it into a PR.
Thanks @mattijn! My understanding is that this line is where the name previously got injected, and so what I tried to do is to remove the "name" property from the encoding at an earlier stage, and then only include it in one spot. Does that make sense?
I will try running the test suite now.
You remove the name from the encoding? In my understanding there is no name key within encoding.
I think what you do is you import the name and define it on the first item in the layered specification, just like we make sure on the Altair side to have name only be defined on the first item in a manual layered specification. So that make sense.
But how you knew you could define name as const around here?
https://github.com/ChristopherDavisUCI/vega-lite/commit/1ced888950ce5580266fc1fc4f35a902238bd449#diff-9d97dcc41d7b274e7a6ad719846874fdd3ebb66ecd70b82dc1ee228e4a598425R104
Which name is this?
You're right, I meant that I remove name from the spec (not from the encoding). This is where that happens: https://github.com/ChristopherDavisUCI/vega-lite/blob/1ced888950ce5580266fc1fc4f35a902238bd449/src/normalize/pathoverlay.ts#L104
The main point is that the name property doesn't get duplicated. Does that make more sense? (If spec does not have a name property, than my JavaScript understanding is that this name will just be undefined.)
Does that make more sense?
Yes it does! Thanks for explaining. I notice there is also an lineOverlay next to pointOverlay. I don't think we had that combination tested yet on the Altair side. So maybe that is another potential-future-issue solved by this change.
Great, thanks @mattijn! I tried making a PR here: #8662. Even if it turns out this isn't the right solution, hopefully it can get us on the right track.