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

Columns not working properly in VL spec (possibly period in name of column?) #1949

Closed
jowens opened this issue Feb 16, 2017 · 17 comments
Closed

Comments

@jowens
Copy link

jowens commented Feb 16, 2017

Have not seen this behavior before. I am trying to make columns from a field with a period in its name; I've done this successfully for other fields before, so maybe that's the culprit? Anyway, column has "field: [gpuinfo.name]" and I have two values for gpuinfo.name, both of which appear in the final plot but all data points, no matter which GPU I choose, are in the same column.

I sincerely hope I just didn't botch my specification somewhere. (This was generated from Altair.)

vega

{
  "mark": "point",
  "data": {
    "values": [
      {
        "engine": "Gunrock",
        "algorithm": "BFS",
        "num_gpus": 1,
        "elapsed": 43.28049087524414,
        "dataset": "soc-LiveJournal1",
        "m_teps": 1563.7744140625,
        "gpuinfo.name": "GPU 1"
      },
      {
        "engine": "Gunrock",
        "algorithm": "BFS",
        "num_gpus": 2,
        "elapsed": 43.28049087524414,
        "dataset": "soc-LiveJournal1",
        "m_teps": 3456.7,
        "gpuinfo.name": "GPU 1"
      },
      {
        "engine": "Gunrock",
        "algorithm": "CC",
        "num_gpus": 1,
        "elapsed": 31.613856554031372,
        "dataset": "soc-LiveJournal1",
        "m_teps": 2710.914794921875,
        "gpuinfo.name": "GPU 2"
      }
    ]
  },
  "encoding": {
    "color": {
      "field": "engine",
      "type": "nominal",
      "legend": {"title": "Engine"}
    },
    "column": {
      "field": "[gpuinfo.name]",
      "type": "nominal",
      "axis": {"orient": "top","title": "GPU"}
    },
    "shape": {
      "field": "engine",
      "type": "nominal",
      "legend": {"title": "Engine"}
    },
    "y": {
      "field": "m_teps",
      "scale": {"type": "log"},
      "type": "quantitative",
      "axis": {"title": "MTEPS"}
    },
    "x": {
      "field": "num_gpus",
      "type": "nominal",
      "axis": {"title": "Number of GPUs"}
    },
    "row": {
      "field": "algorithm",
      "type": "nominal",
      "axis": {"title": "Primitive"}
    }
  }
}
@kanitw
Copy link
Member

kanitw commented Feb 16, 2017

Still haven't got the solution, but in any case this simpler spec produces the same problem

{
  "mark": "point",
  "data": {
    "values": [
      {
        "engine": "Gunrock",
        "algorithm": "BFS",
        "num_gpus": 1,
        "elapsed": 43.28049087524414,
        "dataset": "soc-LiveJournal1",
        "m_teps": 1563.7744140625,
        "gpuinfo.name": "GPU 1"
      },
      {
        "engine": "Gunrock",
        "algorithm": "BFS",
        "num_gpus": 2,
        "elapsed": 43.28049087524414,
        "dataset": "soc-LiveJournal1",
        "m_teps": 3456.7,
        "gpuinfo.name": "GPU 1"
      },
      {
        "engine": "Gunrock",
        "algorithm": "CC",
        "num_gpus": 1,
        "elapsed": 31.613856554031372,
        "dataset": "soc-LiveJournal1",
        "m_teps": 2710.914794921875,
        "gpuinfo.name": "GPU 2"
      }
    ]
  },
  "encoding": {
    "column": {
      "field": "[gpuinfo.name]",
      "type": "nominal",
      "axis": {"orient": "top","title": "GPU"}
    },
    "y": {
      "field": "m_teps",
      "scale": {"type": "log"},
      "type": "quantitative",
      "axis": {"title": "MTEPS"}
    },
    "x": {
      "field": "num_gpus",
      "type": "nominal",
      "axis": {"title": "Number of GPUs"}
    }
  }
}

@jowens
Copy link
Author

jowens commented Feb 16, 2017

Relieved to hear this is actually an issue. I try to keep my Github-issues "actual problem" to "my bad programming skills" ratio above one.

@kanitw
Copy link
Member

kanitw commented Feb 16, 2017

The problem is that any aggregate field would normally get <fn>_ prefix.

In this case, the layout need to know number of unique values but got distinct_[gpuinfo.name]instead (but the field is actually named distinct_gpuinfo.name thus the output should be [distinct_gpuinfo.name]

Straightforward fix would be to make field() method in fielddef.ts output [] for escaping only if necessary and if so, add it around the prefix as well.

Alternatively, we could

  1. modify the field() method so that it outputs escaped names for field with [] when there is an aggregate function (e.g., distinct_gpuinfo_name)
  2. Make the output aggregate transform in assemble() in summary.ts always explicit specify as array.

In any case, we need to test that this works for

  • nominal fields (which affects layout)
  • aggregated quantitative fields
  • binned field
  • temporal fields with timeUnit
  • Make sure that we do not wrap output with datum[] manually
    • Searching for datum[ in the project, I found so many occurrences that have this problem.

@kanitw
Copy link
Member

kanitw commented Feb 16, 2017

#1951 is a start for this (that works for the example above), but we still need to make test pass and test all cases in the previous comment.

@kanitw
Copy link
Member

kanitw commented Feb 16, 2017

@willium wondering if you can help fix this. You're the boss of field() method anyway :)

@kanitw
Copy link
Member

kanitw commented Feb 16, 2017

  • Make this spec pass.
{
  "$schema": "https://vega.github.io/schema/vega-lite/v2.json",
  "description": "A simple bar chart with embedded data.",
  "data": {
    "values": [
      {"a.a": "A","b.b": 28}, {"a.a": "B","b.b": 55}
    ]
  },
  "mark": "bar",
  "encoding": {
    "x": {"field": "[a.a]", "type": "ordinal"},
    "y": {"field": "[b.b]", "type": "quantitative"}
  }
}

@kanitw kanitw modified the milestone: 2.0.0-pre Important Feature & Patches Feb 17, 2017
@kanitw kanitw modified the milestones: 2.0.0-β Important Feature & Patches, 2.0.0-rc Moderately Important Patches Apr 5, 2017
@kanitw kanitw modified the milestones: 2.1 Important Patches, 2.0.0-rc Moderately Important Patches Jun 19, 2017
@kanitw
Copy link
Member

kanitw commented Aug 14, 2017

See #1951 (comment) for an earlier, unfinished attempt.

@domoritz
Copy link
Member

Related: #2625.

@willium
Copy link
Member

willium commented Aug 14, 2017

Are they related? the dash seems to work.

@willium
Copy link
Member

willium commented Aug 14, 2017

I've isolated this down to a problem (or edge case) in Vega:
datum[\"[b.b]\"] doesn't work, but datum[\"b\\.b\"] does.
The square-bracket escaping doesn't work inside of a key name.

@willium
Copy link
Member

willium commented Aug 14, 2017

I'm not sure if we'd call this a bug in Vega or an edge case we didn't account for in Vega-Lite... If it's not a bug, we should improve the docs here.

@kanitw
Copy link
Member

kanitw commented Aug 14, 2017 via email

@willium
Copy link
Member

willium commented Aug 14, 2017

@jheer: In Vega, the escape pattern datum[\"[b.b]\"] doesn't seem to work, while datum[\"b\\.b\"] does. Is this a bug in Vega escaping the dot notation when used as an argument to datum, or perhaps the expected behavior that's not documented in the Vega docs? (A third, equally possible option is that I'm doing something wrong...)

@willium
Copy link
Member

willium commented Aug 14, 2017

@kanitw, @domoritz: in the meantime, I'll convert our default escape pattern to //. so the above spec will work.

@jheer
Copy link
Member

jheer commented Aug 14, 2017

Hmm, this seems to work: vega.field("datum[\"[b.b]\"]")({datum: {"[b.b]": 7}})

But note that the internal brackets are considered part of the key!

@kanitw kanitw modified the milestones: 2.x? Important Patches, 2.x Data & Transforms Patches Sep 22, 2017
@kanitw
Copy link
Member

kanitw commented Dec 5, 2017

@domoritz -- I guess this is already supported with proper escaping? If so, please close the issue.

@domoritz
Copy link
Member

domoritz commented Dec 5, 2017

Yes, it works with escaping.

{
  "mark": "point",
  "data": {
    "values": [
      {
        "engine": "Gunrock",
        "algorithm": "BFS",
        "num_gpus": 1,
        "elapsed": 43.28049087524414,
        "dataset": "soc-LiveJournal1",
        "m_teps": 1563.7744140625,
        "gpuinfo.name": "GPU 1"
      },
      {
        "engine": "Gunrock",
        "algorithm": "BFS",
        "num_gpus": 2,
        "elapsed": 43.28049087524414,
        "dataset": "soc-LiveJournal1",
        "m_teps": 3456.7,
        "gpuinfo.name": "GPU 1"
      },
      {
        "engine": "Gunrock",
        "algorithm": "CC",
        "num_gpus": 1,
        "elapsed": 31.613856554031372,
        "dataset": "soc-LiveJournal1",
        "m_teps": 2710.914794921875,
        "gpuinfo.name": "GPU 2"
      }
    ]
  },
  "encoding": {
    "column": {
      "field": "gpuinfo\\.name",
      "type": "nominal",
      "axis": {"orient": "top","title": "GPU"}
    },
    "y": {
      "field": "m_teps",
      "scale": {"type": "log"},
      "type": "quantitative",
      "axis": {"title": "MTEPS"}
    },
    "x": {
      "field": "num_gpus",
      "type": "nominal",
      "axis": {"title": "Number of GPUs"}
    }
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants