search

Quansight / ibis-vega-transform

@vega transforms with @ibis-project expressions
Apache License 2.0
30 stars 7 forks source link

Fix padding issues #64

Closed saulshanabrook closed 4 years ago

saulshanabrook commented 4 years ago

There seems to be some issues on the current version with padding. There isn't enough of it!

Potentially relevant line: https://github.com/Quansight/ibis-vega-transform/blob/ff6d1eedf72816ae3603d21d4eb6faccc2f1032a/src/renderer.ts#L100-L104

And potentially relevant links: https://vega.github.io/vega/docs/api/view/#view_resize https://vega.github.io/vega/docs/specification/#autosize (thanks @domoritz)

We should determine how altair is doing it and if they have the same issue.

domoritz commented 4 years ago

If the same issue exists in Altair, we may have to generate different Vega from Vega-Lite.

saulshanabrook commented 4 years ago

I can't seem to reproduce this in master. It's possible this was fixed when we upgraded all versions? Is anyone else still seeing this issue?

saulshanabrook commented 4 years ago

Ah I figured out this happens because the vega view initially loads with no data, and then when the data comes in, the size changes. Sometimes this causes then some parts to be out of the view and cut off. This seems to be fixed by configuring the chart with resize:

alt.Chat(....).configure(
    autosize={
        'resize': True
    }
)
domoritz commented 4 years ago

Which adds a bit of runtime overhead. You can also call view.resize() when the data has been loaded. I don't think it makes a significant difference in this case, though.

saulshanabrook commented 4 years ago

@domoritz thanks yeah, was thinking of that as well. I can add that if we encounter problems!

domoritz commented 4 years ago

"Make it work, make it right, make it fast." - Kent Beck