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

Vega 2 Sort #612

Merged
merged 12 commits into from
Oct 7, 2015
Merged

Vega 2 Sort #612

merged 12 commits into from
Oct 7, 2015

Conversation

kanitw
Copy link
Member

@kanitw kanitw commented Oct 7, 2015

(part of #459)

  • remove sort.js and no longer use sort-<field_name> tables as data source
  • modify scale.domain() to include DataRef’s .sort object
  • modify sort property to be consistent with Vega 2 (but still leave it as a top-level vega-lite property)
    • sort.name => sort.field
    • sort.aggregate => sort.op
  • update tests

- modify scale.sort() to produce scale's sort object
- modify sort property to be consistent with Vega 2  (TODO: consider if we should move `sort` to become scale's property -- to be consistent with Vega2)
@kanitw kanitw mentioned this pull request Oct 7, 2015
13 tasks
@kanitw kanitw changed the title [WIP] Vega 2 Sort Vega 2 Sort Oct 7, 2015
@kanitw kanitw changed the title Vega 2 Sort [WIP] Vega 2 Sort Oct 7, 2015
@kanitw kanitw changed the title [WIP] Vega 2 Sort Vega 2 Sort Oct 7, 2015
@kanitw
Copy link
Member Author

kanitw commented Oct 7, 2015

Actually there are still some incorrect cases for movies.json:

A) Dot Plot

{
  "marktype": "point",
  "encoding": {
    "x": {
      "name": "MPAA_Rating",
      "type": "N"
    }
  },
  "data": {
    "url": "data/movies.json"
  }
}

although the output scales looks correct

...
"scales": [
        {
          "name": "x",
          "type": "ordinal",
          "domain": {
            "data": "raw",
            "field": "MPAA_Rating",
            "sort": true
          },
          "range": [0,189],
          "bandWidth": 21,
          "round": true,
          "nice": true,
          "points": true,
          "padding": 1
        }
      ],
...

The output visualization does not order the scale correctly

vega-lite_editor

B) Faceted Small Multiples

{
  "marktype": "point",
  "encoding": {
    "x": {"name": "Rotten_Tomatoes_Rating","type": "Q"},
    "y": {"name": "IMDB_Rating","type": "Q"},
    "col": {"name": "MPAA_Rating","type": "N"}
  },
  "data": {"url": "data/movies.json"}
}

The col scale has sort=true

   {
      "name": "col",
      "type": "ordinal",
      "domain": {"data": "raw","field": "MPAA_Rating","sort": true},
      "bandWidth": 150,
      "round": true,
      "nice": true,
      "padding": 0.1,
      "outerPadding": 0
    }

but the output is not well sorted

vega-lite_editor

@kanitw
Copy link
Member Author

kanitw commented Oct 7, 2015

However, similar visualizations from barley.json produces well-sorted output.

A) Dot Plot

{
  "marktype": "point",
  "encoding": {
    "x": {"name": "variety","type": "N"}
  },
  "data": {"url": "data/barley.json"}
}

vega-lite_editor

B) Small Multiple

{
  "marktype": "point",
  "encoding": {
    "x": {"name": "site","type": "N"},
    "y": {"name": "yield","type": "Q"},
    "col": {"axis": {"maxLabelLength": 25},"name": "variety","type": "N"}
  },
  "data": {"url": "data/barley.json"}
}

vega-lite_editor

Not sure what makes things works correctly for barley.json, but not for movies.json

I have inspected barley.json and make sure that the original data is not pre-sorted by variety.

@kanitw
Copy link
Member Author

kanitw commented Oct 7, 2015

Actually some variables in movies.json works correctly – for example, Creative_Type works okay.

But some like Major_Genre, MPAA_Rating, Source, Title are broken …

Not sure what makes these sorting incorrect but this sounds more like a bug in lower-level stacks.

domoritz added a commit that referenced this pull request Oct 7, 2015
@domoritz domoritz merged commit 1a81f4b into 0.8 Oct 7, 2015
@domoritz domoritz deleted the kw/vg2-sort branch October 7, 2015 17:23
@kanitw
Copy link
Member Author

kanitw commented Oct 8, 2015

Thanks for merging. I just appended changes to 0.8 branch.

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

Successfully merging this pull request may close these issues.

2 participants