Open charts in the default browser with open_editor method

[joelostblom]

I have been working more in VS Code instead of JupyterLab lately and really come to appreciate the to_url() method added in #3252 (thanks @jonmmease!). VS Code can't correctly handle the Vega-Lite actions menu #2875 and there seems to be little hope of fixing this microsoft/vscode-jupyter#13261.

However, after trying to troubleshoot a few altair specs in the Vega-Lite window, I find myself copying the url over and over again after small edits to the altair spec (for me VS Code only sometimes figures out that the url is a link that can be clicked; maybe related to the length of the spec). It would be convenient if the Vega-Lite spec of the charts could automatically open in the default browser, which is what this PR tries to implement (also mentioned in #3252 (comment)).

I put the browser launch as the default when using to_url(), since I imagine that the most common scenario is to quickly go between the local editor and the online Vega-Editor to troubleshoot a spec. I find this convenient, but if it is considered too aggressive, then I'm happy to change it.

Tagging everyone who was involved in the previous PR discussion: @jonmmease @NickCrews @mattijn @binste

[joelostblom]

This technically breaks if someone used to rely on fullscreen as a positional argument via to_url(True). I don't think it's a huge deal, but also have nothing against reversing the order.

[binste]

Good thing is that thanks to the *, fullscreen has always been a keyword-only argument so this should be fine.

[NickCrews]

This is why we should lean towards kwarg-only APIs when we have the choice :) yay thanks past us

[joelostblom]

This is cool, I haven't come across this use of * to block additional positional args. Thanks for linking the pep!

[binste]

Same! I use VS Code a lot and often use .to_url to debug a spec in the Vega Editor. I'm fine with keeping it simple and having open_browser=True as the default. I can't think of another use case for the to_url method but maybe we're missing something?

@joelostblom Could you also add a note in the changelog?
Code looks good to me. Let's see if anyone else has an opinion on this before merging.

[jonmmease]

Thanks for the PR @joelostblom, I'm glad the to_url functionality has been helpful!

I think I'm -1 on overloading to_url this way, because I expect to_* methods to perform conversions, not have side effects. And when the goal is to see the chart in the editor, you probably don't want the function to return the full URL and clutter up the notebook output. Also, if goal is to open chart in the editor, the fact that this is done using a URL is more of an implementation detail. e.g. Maybe someday we add the option to create a gist and open the editor pointing there.

So, I'd suggest adding a new top-level chart method named something like chart.open_editor(fullscreen=False) that calls to_url internally and applies the logic in this PR for opening the chart in the browser. We could even expose the other webbrowser.open options in the signature if we want (https://docs.python.org/3/library/webbrowser.html#webbrowser.open). We could also use webbrowser.get to support specifying the name of the browser to use. This doesn't need to be in this PR, just thinking that there are a bunch of additional options folks may want eventually, and so I think a dedicated method is warranted.

[NickCrews]

I think I agree with @jonmmease , I'm -.5 on this.

Would it be way too clever if we made it so that to_url() returned a

class VegaUrl(str):
    def open(self): ...

?

This is similar to what some SQL libraries do, they return an object that ducks like a str, but has some additional features.

Using this would look like chart.to_url().open()

[joelostblom]

Thanks for the input everyone!

I think I'm -1 on overloading to_url this way, because I expect to_* methods to perform conversions, not have side effects. And when the goal is to see the chart in the editor, you probably don't want the function to return the full URL and clutter up the notebook output. Also, if goal is to open chart in the editor, the fact that this is done using a URL is more of an implementation detail. e.g. Maybe someday we add the option to create a gist and open the editor pointing there.

All great points. I was initially thinking that it seemed unnecessary with yet another method, but after considering what you wrote I think it makes more sense to go down that path.

I don't have a strong preference between chart.open_editor() and chart.to_url().open(). The former seems more discoverable whereas the latter looks neatly composable. What does everyone else think? Two somewhat related points regarding syntax:

• If we go with the to_url().open() approach, should we also add this to the save method? E.g. chart.save('chart.html').open() could be opened the same way as here (and maybe images/PDFs could also with a file:// URL?).
• We could think about what the syntax for How to show altair charts from console programs? #3330 should be. I'm guessing we will keep it as chart.show() and not go with something like chart.to_html().open()? We also have chart.display() for displaying via ipython, which doesn't use an .open() approach, so although I quite like the composability, maybe it is too different from what is in Altair already (although we could of course change that if we want).

@joelostblom Could you also add a note in the changelog?

Yup, will do!

[binste]

Good points, now I'm also in favour of putting it into a new method :)

I like the custom string classes in SQL libraries! However, I'm not sure if in Altair we'd have a use case other than this for it so it might be a bit unexpected to users and a simple .open_editor could be more straightforward to understand. A URL seems like a general concept which has not much to do with the rest of the library. That's only a slight preference.

[binste]

Removing the "Approve" for now as there seems a consensus to use a different method than to_url

[jonmmease]

I prefer open_editor to VegaUrl for discoverability and for the sake of auto-generated API docs, but it is a clever idea!

[joelostblom]

Thanks for the input everyone. I also have a slight preference for open_editor for discoverability and consistency with the other methods I mentioned. Since most of us are leaning in that direction I went ahead and updated the PR accordingly. I can't think of any relevant tests to add and we wouldn't want the browser to actually open when running the test suite (maybe there is some clever way around that but probably too much for this small addition?).

[NickCrews]

LGTM!

[jonmmease]

lgtm. Could you update the PR title?

Regarding testing, we could do something clever with monkey patching the webbrowser module. But this is so simple I don't think it's necessary.

[joelostblom]

@binste Just a ping in case you wanted to have a second look too; otherwise I'm fine to merge.

[binste]

LGTM, thank you @joelostblom!