-
Notifications
You must be signed in to change notification settings - Fork 794
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
Support converting a Chart to a Vega editor URL #3252
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @jonmmease. Not sure if that was the intention but it helps with #2875 which is nice.
Only added two minor comments.
Co-authored-by: Stefan Binder <[email protected]>
Co-authored-by: Stefan Binder <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thank you!
Could consider adding auto copy to clipboard support, but I lean towards that being out of scope
altair/vegalite/v5/api.py
Outdated
@@ -1055,6 +1055,25 @@ def to_html( | |||
requirejs=requirejs, | |||
) | |||
|
|||
def to_url(self, fullscreen: bool = False) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider: forcing all args to be keyword-only so that in the future, if we add more params to this function, they are not order dependent and we can re-order them as needed without breaking anyone. Not sure if this is overkill.
to_url(False) is also not very obvious what False does. Requiring a kwarg improves that.
On the other hand, requiring a kwarg is more typing.
I think I fall 60% in favor of requiring kwarg but am fine with keeping it as is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea! Done in 5d440f9
Yeah, this is a possible future extension. Another idea is to optionally use the Python webbrowser module to open the URL in the editor. |
Thanks for the PR @jonmmease and reviews @NickCrews, @binste! This is great and valuable (especially in VSCode, like Stefan said.) |
Closes #3188
This PR adds a
Chart.to_url
method that uses VlConvert to generate a Vega editor URL that opens the chart's specification in the online Vega editor.https://vega.github.io/editor/#/url/vega-lite/N4Igxg9gdgZglgcxALlANzgUwO4tJKAFzigFcJSBnAdTgBNCALFAZgAY2AacaYsiygAlMiRoVYcAvpO50AhoTl4QpAE4AbFCDGEADpWQB6Q2DpQAdACtKdTOrhpV5qJkKGougLaG0mBHIBaeUVKV0oAATQARnMAJgBOczZDYLkTOVVKK0poEBkQTwyAa2VCAE9dTC1dCBJxfMwoSDoSJFQedQhVZXg7Oi0AeVVEEhBucsqtKAhPEjlNfIAPHqx1fuQQQS7QmuxMbvGKqo2AR1I5IjhFYl887jKVvq0AWTh1TEoAfUrVT4BxeadKBjEATY4gM4XYjXBxVaTcAAklDAjEwhS0On0Rh8fjk5gQV0YpAARuY4BBDMjUYUcf4AvZCJgfABWcxRVkxay5SRAA
cc @NickCrews