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

Tidy line chart examples #2644

Merged
merged 9 commits into from
Jul 5, 2022
Merged

Conversation

palewire
Copy link
Contributor

@palewire palewire commented Jul 3, 2022

This proposal is a revival of #1284, which lost its downstream fork and had conflicts requiring resolution. I believe it is ready now for reconsideration.

I've added two new line chart examples lifted from the ones upstream in the Vega Lite docs. In the interest of uniformity, I also modified two existing examples to ensure that they all are drawn from the same datasets as complimentary examples, which will make things quicker to grok for newbies.

@joelostblom
Copy link
Contributor

I think these additional examples are useful! Could we change the dataset in all these examples to the stocks data instead so that the axis can be temporal instead of ordinal? It looks like this is what they have done in the Vega-Lite repo as well https://vega.github.io/vega-lite/examples/#line-charts for the corresponding line charts.

@palewire palewire mentioned this pull request Jul 5, 2022
@palewire
Copy link
Contributor Author

palewire commented Jul 5, 2022

Alright. I think a good chunk of what we had here was handled upstream. But, FWIW, I think what remains is still worth merging.

Copy link
Contributor

@joelostblom joelostblom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

@joelostblom joelostblom merged commit 160ab03 into vega:master Jul 5, 2022
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