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

Feature: Shareable link to eg https://vega.github.io/editor #3188

Closed
NickCrews opened this issue Sep 13, 2023 · 8 comments · Fixed by #3252
Closed

Feature: Shareable link to eg https://vega.github.io/editor #3188

NickCrews opened this issue Sep 13, 2023 · 8 comments · Fixed by #3252

Comments

@NickCrews
Copy link
Contributor

I filed the main issue at vega/editor#1365 because I thought that was a better place for it, but just posting this here for visibility. Please chime in there with your thoughts!

@mattijn
Copy link
Contributor

mattijn commented Sep 13, 2023

I think it is a good feature request. I would say it is useful. This requires a synergy with the Vega-editor though and we should be sure that for the foreseeable future this keeps working.

If we try to do this, we should aim for not introducing new hard dependencies, but a soft dependency is acceptable I think.

Btw, I also often publish my Altair derived VL-specification as gist on GitHub and refer to this in the Vega-editor. Than the shareable-url is much shorter than the LZW compressed string.

We've discussed before to have a uniform approach for export methods. I opened #3189 to keep track on this. I think your suggestion of .to_url() fits in there as well.

@NickCrews
Copy link
Contributor Author

a synergy with the Vega-editor

I agree. Definitely requires buy in from them. Please leave a comment there in support of this so they know that altair folks are interested!

gist on GitHub

This works in many cases, but not for me, because then the data is public, and I need to keep it private.

@jonmmease
Copy link
Contributor

This is a neat idea @NickCrews. I opened an issue in VlConvert about the idea of using a Rust implementation of lz-string to perform the URL encoding: vega/vl-convert#103.

VlConvert is what Altair already uses by default for svg and png image export, so we could potentially add editor URL support using the same optional dependency.

@NickCrews
Copy link
Contributor Author

NickCrews commented Sep 13, 2023

nice @jonmmease ! That is one path forward. Ideally though, I would rather have everyone using a more standard compression algorithm so

  1. We aren't relying on tiny unpopular packages with maybe bad maintenance
  2. It is easy for other programming languages/projects to implement

See my prototype using brotli in the editor issue.

@NickCrews
Copy link
Contributor Author

Following some updates in the linked editor issue, I think I've changed my stance a bit. I think fixing the existing python port of lz-string and adding it to Altair makes sense. It will only be one small python file, and we can always add the brotli stuff later.

If I fix the lz-string port to python, I see a few paths forward:

  1. Vendor it directly into Altair
  2. Distribute it on pypi as a package. That is a service to the community and keeps altairs code smaller. Maybe not worth it though. Could either be optional depending, or required. since it is only one pure python file making it a hard dep doesn't seem bad?
  3. Fix up a rust implementation and then use pyo3 to make python bindings. If we cared about performance then this would be good, but I don't think we do. Much more complex.

What would you prefer @mattijn ?

@NickCrews
Copy link
Contributor Author

Or wait, I guess we could just use vlconvert directly? Should I write a PR that does this?

@jonmmease
Copy link
Contributor

@NickCrews, yeah I was picturing that we would use VlConvert for this the way we currently use it for svg/png image export. I'm expecting to do a VlConvert release toward the end of next week and I'll ping you when it's available. It would be great if you wanted to work on an Altair PR to add the chart.to_url() method using VlConvert. I'll also be making a PR to add support for saving charts to PDFs, so I could include the to_url support at that point as well.

@NickCrews
Copy link
Contributor Author

If you are already in there adding to_pdf(), I would love to avoid learning how to set up the dev environment, tests, etc. So go for it! I can help review if you tag me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants