Setting only width when using autosize: fit should not use default height in compiled Vega

[willium]

See the compiled Vega from this example. The height is set the default config.view.height of 200, but I'd expect this chart to only "fit" the width (since that is all I specified).

To solve this, I imagine there are two options:

1. We should not apply the config's height or width when using "autosize": "fit" with one of the dimensions specified.
2. We allow for some value of "autosize" that specifies only to "fit" along one dimension.

[kanitw]

2. would require changing Vega. Maybe 1. is simpler to do?

[kanitw]

Unfortunately, I was playing with it a little bit. The problem is we still need to step the height to step * cardinality, but then Vega will try to fit axis into the frame with height = step * cardinality, which doesn't work. See this spec.

Thus, 1. is not feasible. If you really want this, we need 2 in Vega. Please file an issue in Vega if you really want this.

[willium]

Thanks for taking a look @kanitw! I created an issue in the Vega repo. I should be able to implement it in Vega-lite when it's supported in Vega.

[kanitw]

I'm wrong lol. There is fit-x/y in Vega: https://vega.github.io/vega/docs/specification/#autosize.

I'd wait until #5138 lands before implementing this to avoid massive conflicts.

[kanitw]

Once that lands, you probably want to:

• Update model.fit to be {x: boolean, y: boolean} instead of boolean and update their references accordingly.
• Document the behavior in size.md

Thanks for volunteering to help. :)

[willium]

So if you just use autofit: fit, would be still have the width/height set by the config defaults, and then if you specify only fitting along one dimension, we only override for that particular dimension?

[kanitw]

So if you just use autofit: fit, would be still have the width/height set by the config defaults

That's a good question. We should consider how autofit should interact with the introduction of step-based width/height as well as
config.view.continuousWidth/Height and config.view.discreteWidth/Height and in #5138.

Consider the example you post above, given the spec says autosize: "fit" and specifies only width, should VL retain the default config.view.discreteHeight and change "fit" to become "fit-x" automatically so that VL still respect the default config? (Probably yes?)

Currently in #5138, I do an awkward thing where we use config.view.continuousHeight for a discrete y-field if fit is specified, which is a bit weird given it is not a continuous height. -- I start thinking that it's more sensible to respect the config.view.discreteHeight and downgrade "fit" to "fit-x" as "y" cannot be fit.

Also what if we get width: 200 and height: {step: 20} with "fit", should we drop "height: {step: 20} or make "fit" become "fit-x"? I think I prefer the latter. But it's worth noting that we are now making width/height have a higher precedence than "fit", which is a bit different from the current behavior which makes "fit" drops rangeStep.

This issue is complicated. It's definitely worth enumerating how all the relevant properties can interact (autosize, explicit width/height, type of x- and y-fields as well as config.view.continuousWidth/Height and config.view.discreteWidth/Height) so we don't overlook other corner cases before we go ahead and implement a design.

[willium]

My current thinking is that we should respect whatever the user declares, and never cleverly downgrade fit to be fit-x or fit-y. That means if they fail to specify the width and use fit rather than fit-y, they get a broken chart. The same goes for step, it wouldn't be overriden in any case.

Do you think we should have the API be autosize: 'fit' | 'fit-x' | 'fit-y' | undefined or something more ergonomic like autosize: 'fit' | {x?: boolean, y?: boolean} | undefined? I slightly prefer the latter, as it affords the option of deprecating 'fit' in a later version of VL, requiring the user be more explicit.

What do you think @kanitw @domoritz?

[kanitw]

@willium Thanks for following up. Here are my thoughts:

My current thinking is that we should respect whatever the user declares and never cleverly downgrade fit to be fit-x or fit-y. That means if they fail to specify the width and use fit rather than fit-y, they get a broken chart.

I agree that we should generally respect whatever the user declares.

However, I disagree that we should intentionally show broken chart when users specify an invalid specification, which would only makes it feels more like a bug to users. There are many places that we drop invalid specification (with a warning explaining why something is dropped) and render something more reasonable as it's friendlier. So I think we should follow the same approach here.

In this case, "fit" would require a number to fit to. If y is discrete (with discreteHeight = {step:...} by default), even if user specify "fit", there is no clear height to fit the y-scale to. (Fitting a discrete axis to the default continuous height also doesn't match what the specification says.)

Thus, it would make sense to drop to fit-y part and throw warning.

The same goes for step, it wouldn't be overriden in any case.

For width=step with fit-x, it's also similarly problematic as we can't fit-x to a step. As "width=step" and "fit-x" are conflicting, we need to define their precedence. Given there is no reasonable fixed width to read from, it would make sense to drop "fit-x" and stick to the step value.

Do you think we should have the API be autosize: 'fit' | 'fit-x' | 'fit-y' | undefined or something more ergonomic like autosize: 'fit' | {x?: boolean, y?: boolean} | undefined? I slightly prefer the latter

Autosize can be "pad" too. {x?: boolean, y?: boolean} doesn't give the impression that this is for fitting x/y. Maybe {fitX?: boolean, fitY?: boolean} is better.

However, "autosize" also already has a different object form in Vega (which I can't remember if we have fully supported yet, but we probably should if we haven't). So maybe 'fit' | 'fit-x' | 'fit-y' | undefined is better as we won't introduce multiple forms of autosize objects.

[willium]

That makes sense. To make sure I have this right: the compiled fit should correspond to the minimal set of explicitly defined dimensions (argmin over the specified fit dimension).
Cases (vl --> vg):

1. "autosize": "fit" and "width": 200 --> fit-x
2. "autosize": "fit-x" and "width": 200 --> fit-x
3. "autosize": "fit-x" and "width": { "step": ... } --> no fit
4. "autosize": "fit-x" and "width": { "step": ... } and "height": 200 --> no fit
5. "autosize": "fit" and "width": { "step": ... } and "height": 200 --> fit-y
6. "autosize": "fit" and "width": 200 and "height": 200 --> fit
7. "autosize": "fit-x" and "width": 200 and "height": 200 --> fit-x
8. "autosize": "fit-y" and "width": 200 and "height": 200 --> fit-x
9. "autosize": "fit" and "width": { "step": ... } and "height": { "step": ... } --> no fit
10. "autosize": "fit" and "width": 200 and "height": 200 and row encoding --> fit-y
11. "autosize": "fit" and "width": 200 and "height": 200 and column encoding --> fit-x

Do all of these seem right @kanitw @domoritz?

[domoritz]

Can you explain to me why we need fit-x in Vega-Lite? Isn't it equivalent to just specifying width and autosize: 'fit'?

Other than that, your list looks right to me.

[willium]

If we do go this way, we could just keep the language as is (only fit or no fit) and just change the compiler to use the proper fit in Vega (and not apply the config width/height)

[domoritz]

Yes. In #5133 (comment), you proposed to change VL, though, right? I am not yet convinced that we need to change the VL language.

[willium]

Yep. We would’ve had to change VL if we did what I suggested in #5133 (comment)

[kanitw]

Can you explain to me why we need fit-x in Vega-Lite? Isn't it equivalent to just specifying width and autosize: 'fit'?

When both x and y-axes are both quantitative, you can still choose to fit only one of them by using either fit-x or fit-y.

I'm not sure when would that be useful, but fit-x and fit-y definitely offers more possibility.

Also, if we throw warning when we reduce "fit" to either fit-x or fit-y, then it's important to provide "fit-x" and "fit-y" , which do not produce warnings.

[willium]

hmm, this complicates things. Are you suggesting that:

"autosize": "fit" on QxQ

compiles to fit in Vega (and apply width/height config)?

then, should

"autosize": "fit" and "width": 200 on QxQ

compile to fit in Vega (and apply height config)?

(note that this is different than the cases described in this comment)

[domoritz]

Ahh, I see that if we use the size from the config, then having fit-x and fit-y offer more flexibility.

[kanitw]

should "autosize": "fit" and "width": 200 on QxQ compile to fit in Vega (and apply height config)?

Yes. Good point that it differs a bit from #5133 (comment).

But we can simplify a lot of logic by saying that

width = specifiedWidth || config.view.xxxWidth // where XXX depends on whether x is discrete or continuous

Then the logic is simply "drop the 'x' part of fit when width (as defined above) is based on a step size".

Note that we drop step when if x is continuous (as it's invalid), so this logic should come after we drop step.

The same logic can be applied for "height" and "y".

I think this actually simplifies the logic as compared to the comment above.
Basically, we shouldn't distinguish the width/height values whether it's explicitly specified or derived from config.

[willium]

if we do this, wouldn't it be impossible to specify something like

{
   "width": 200, 
   "autosize": "fit-x"
}

without vega-lite adding in the default height as well? Or is that intentional?

[domoritz]

Of I understand @kanitw's comment correctly, he assumes that we add fit-x and fit-y.

[kanitw]

if we do this, wouldn't it be impossible to specify something like

{
"width": 200,
"autosize": "fit-x"
}

without vega-lite adding in the default height as well? Or is that intentional?

Vega-Lite already always add default width/height if not specified (always a number for continuous and number or step for discrete) as in:

width = specifiedWidth || config.view.xxxWidth 

Basically, the width/height always have to come from somewhere anyway. So this issue is about how to apply the width/height (to just the axis or fit to axis + padding for label/title), not about how we get width/height.

width = specifiedWidth || config.view.xxxWidth

Of I understand @kanitw's comment correctly, he assumes that we add fit-x and fit-y.

Yes, my proposed scheme works better with width and height.

[willium]

Currently, fit has precedence over step (step is dropped). However, I believe taking this approach, with reverse this precedence:

Then the logic is simply "drop the 'x' part of fit when width (as defined above) is based on a step size".

Is this alright?

An alternative to maintain some precedence over step:

1. { "autosize": "fit", "width": { "step": 20 }, "height": 500}
   → fit-y (drop x component of fit)
2. { "autosize": "fit-x", "width": { "step": 20 }, "height": 500}
   → drop step (keep fit-x)

[kanitw]

Currently, fit has precedence over step (step is dropped ). However, I believe taking this approach will reverse this precedence.

Yes, we should change the old precedence, which is simply a leftover from the old code (before width.step exists) -- but not intentionally designed precedence.

As discussed above:

For width=step with fit-x, it's also similarly problematic as we can't fit-x to a step. As "width=step" and "fit-x" are conflicting, we need to define their precedence. Given there is no reasonable fixed width to read from, it would make sense to drop "fit-x" and stick to the step value.

To expand, if you drop explicitly specified step, you still need to read the width from somewhere to fit to the width. But the default discrete width is usually a step too, so you don't have a fixed width to read from. Thus, it's better to drop x part of "fit" when there is a step.

For the cases, you describe:

An alternative to maintain some precedence over step:

1. { "autosize": "fit", "width": { "step": 20 }, "height": 500}
   → fit-y (drop x component of fit)
2. { "autosize": "fit-x", "width": { "step": 20 }, "height": 500}
   → drop step (keep fit-x)

I think 1. make sense.

For 2., I think fit step should be dropped. Otherwise, it's unclear what width should we fit-x to. (Given the default discrete width is also a step.)

[willium]

In case 1, step has precedence, and in case 2, fit has precedence. Do you think that'd be confusing?

[kanitw]

In case 1, step has precedence, and in case 2, fit has precedence. Do you think that'd be confusing?

Yes. (Sorry, I just realize I have a typo -- "fit" (not step) should be dropped. -- corrected above.) So "fit" should be dropped in both cases.

Note that we currently drop step and use the width from continuousWidth even though the axis is discrete. This is obviously a bug.