haifengl / smile

Statistical Machine Intelligence & Learning Engine
https://haifengl.github.io
Other
5.97k stars 1.13k forks source link

Incorrect spec generated for encoding channel sort #764

Closed markuspeters closed 2 months ago

markuspeters commented 3 months ago

In smile-scala 3.1.0 this:

VegaLite.facet(
  jsan"""[
    {"day": "Wednesday", "v": 28},
    {"day": "Monday", "v": 55},
    {"day": "Tuesday", "v": 66}
  ]""").
  mark("rect").
  x(field = "day", `type` = "nominal", 
    sort = Some("""["Monday", "Tuesday", "Wednesday", "Thursday", "Friday"]""")).
  y(field = "v", `type` = "ordinal")

is converted to the following encoding channel spec:

"x": {
  "field": "day",
  "sort": "[\"Monday\", \"Tuesday\", \"Wednesday\", \"Thursday\", \"Friday\"]",
  "type": "nominal"
}

The escaped JsString for sort in the spec above should instead be a JsArray of JsStrings, i.e.:

  "sort": ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday"]

I'll be happy to try submit a PR if that seems useful.

haifengl commented 3 months ago

sort is currently Option[String]. In this case, you want to pass an array. We may need to chnage sort to JsValue. Yes, your PR is very welcome. Thanks!

haifengl commented 3 months ago

I have revised the API to take JsValue for sort and a few other simplifications. Will push to master soon.

markuspeters commented 3 months ago

Thank you for handling this, I appreciate the quick change!

haifengl commented 2 months ago

Note that we remove unnecessary factory methods taking data parameter. VegaLite.view()/facet() now take optional title parameter. Call data() methods to specify data source or inline data.