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

Binned x/y encoding ignores scale domain parameter #3575

Closed
jheer opened this issue Mar 27, 2018 · 5 comments
Closed

Binned x/y encoding ignores scale domain parameter #3575

jheer opened this issue Mar 27, 2018 · 5 comments

Comments

@jheer
Copy link
Member

jheer commented Mar 27, 2018

When using a binned spatial encoding, custom scale domain settings seem to be ignored. The generated Vega pulls the scale domain from the data, not the provided custom domain.

Here is an example spec:

{
  "$schema": "https://vega.github.io/schema/vega-lite/v2.json",
  "data": {"values": [0.1, 0.6, 2.3]},
  "mark": "bar",
  "encoding": {
    "x": {
      "field": "data",
      "type": "quantitative",
      "bin": {"extent": [0, 10], "step": 0.5},
      "scale": {"domain": [0, 10]}
    },
    "y": {
      "aggregate": "count",
      "type": "quantitative",
      "scale": {"domain": [0, 2]}
    }
  }
}

I expect the x-axis domain to be [0, 10], but instead it is [0, 2.5].

@kanitw
Copy link
Member

kanitw commented Apr 7, 2018

@yhoonkim I remember you work on this.

Can you remember why setting bin scale's domain is prevented?

https://github.com/vega/vega-lite/blob/master/src/compile/scale/domain.ts#L177

(It should be relatively easy to fix, but wanna understand if fixing doesn't cause other bad effects / regression.)

@yhoonkim
Copy link
Contributor

yhoonkim commented Apr 9, 2018

@kanitw

I guess we prefer to control the scale by bin.extent instead of scale.domain.

But it's kinda weird that one of the bin parameter is outside of the bin object. (It's also more than just domain as it affects the bin transform that we perform before the scale.)
(#2791 (comment))

However,
it is still a problem when scale.domain and bin.extent are specified together.

My suggestion is ignoring scale.domain when the field is binned and throwing a warning message.
(#2429)

So we ignored scale.domain and force to use bin.extent with a warning message. + We decided to fix it later...

--

The above Jeff's spec generates x scale in vega spec like the below:

{
      "name": "x",
      "type": "linear",
      "domain": {
        "data": "data_0",
        "fields": [
          "bin_extent_0_10_step_0_5_data",
          "bin_extent_0_10_step_0_5_data_end"
        ]
      },
      "range": [
        0,
        {
          "signal": "width"
        }
      ],
      "zero": false
    }

However, bin_extent_0_10_step_0_5_data_end is 2.5 so it makes the x-domain [0,2.5]. (It is also related to a standard aggregation behavior [that] (vega/vega#983).) Thus, I think we have to keep using bin.extent as the domain but set vega's x-domain to be the user defined value ([0, 10]) instead of the value from the calculation ([0, 2.5).

@kanitw
Copy link
Member

kanitw commented Apr 22, 2018

@yhoonkim can you help fix this then?

@yhoonkim yhoonkim self-assigned this Apr 22, 2018
@yhoonkim
Copy link
Contributor

yhoonkim commented Apr 22, 2018

@kanitw I'll try! To solve this, I need to hear your thoughts about the below specs and their outcomes. Could you tell me if they look OK?

  1. Spec
{
    "$schema": "https://vega.github.io/schema/vega-lite/v2.json",
    "data": {"values": [0.1, 0.6, 2.3]},
    "mark": "bar",
    "encoding": {
      "x": {
        "field": "data",
        "type": "quantitative",
        "bin": {"extent": [0, 10], "step": 2},
        "scale": {"domain": [-1, 3]}
      },
      "y": {
        "aggregate": "count",
        "type": "quantitative",
        "scale": {"domain": [0, 2]}
      }
    }
  }

image
(In my opinion, the visualization looks bad, but the outcome of the spec is fine. I think Vega-Lite doesn't need to correct this bad, and users should be careful when specifying bin.extent and scale.domain together. )

  1. Spec
{
    "$schema": "https://vega.github.io/schema/vega-lite/v2.json",
    "data": {"values": [0.1, 0.6, 2.3]},
    "mark": "bar",
    "encoding": {
      "x": {
        "field": "data",
        "type": "quantitative",
        "bin": {"extent": [0, 10], "step": 2},
        "scale": {"domain": [-10, 20]}
      },
      "y": {
        "aggregate": "count",
        "type": "quantitative",
        "scale": {"domain": [0, 2]}
      }
    }
  }

image

(In my opinion, It looks OK. If users want to draw ticks, they should specify bin.extent as the same as scale.domain. )

@kanitw kanitw added this to the 2.x Visual Encoding Patches milestone Apr 22, 2018
@kanitw
Copy link
Member

kanitw commented Apr 25, 2018

  1. is ok

  2. is a bit weird that the axis values is only drawn from the bin value. In a way, if users customize scale.domain, then this is okay.

The problem is that axis's "values" is set to

"values": {
        "signal": "sequence(bin_extent_0_10_step_2_data_bins.start, bin_extent_0_10_step_2_data_bins.stop + bin_extent_0_10_step_2_data_bins.step, bin_extent_0_10_step_2_data_bins.step)"
      },

I think it's better if the start and stop are replaced with domain min and domain max.

image

but I'm not sure how to do that in Vega expression and maybe it's okay to just drop "values" in such case.

image

  1. More importantly, this reminds me that users shouldn't have to set domain in addition to extent.

We should make scale domain depends on explicitly set bin's extent too. (This is even more important than correcting axis's "values".)

Basically,

{
    "$schema": "https://vega.github.io/schema/vega-lite/v2.json",
    "data": {"values": [0.1, 0.6, 2.3]},
    "mark": "bar",
    "encoding": {
      "x": {
        "field": "data",
        "type": "quantitative",
        "bin": {"extent": [-10, 10], "step": 2}
      },
      "y": {
        "aggregate": "count",
        "type": "quantitative",
        "scale": {"domain": [0, 2]}
      }
    }
  }

should have the domain = [-10, 10] nicely like this:

image

(I guess this isn't too magical and should be a reasonable default?)

kanitw pushed a commit that referenced this issue Apr 28, 2018
Fix #3575

- If `scale.domain` is specified, use `scale.domain` despite `bin`.
- If `scale.domain` is specified, axis' ticks are ranged over `scale.domain` instead of the signal of the `bin.extent`.
- If `bin.extent` is specified, the scale domain is `bin.extent` by default.
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

4 participants