jwass / mplleaflet

Easily convert matplotlib plots from Python into interactive Leaflet web maps.
BSD 3-Clause "New" or "Revised" License
521 stars 76 forks source link

hack to avoid cross-origin pb in notebooks #20

Closed BibMartin closed 9 years ago

BibMartin commented 9 years ago

Issue

I often work with a distant IPython notebook / jupyter server ; in that case, there are cross-origin issues in the browser, as leaflet.css and leaflet.js cannot be loaded.

Fix

I fix it in using urllib2 and asking python to download and serve these files. There may be cleaner ways to do that ; especially in adapting the template.

Bonus

And I take the occasion to put the rendered figure in an iframe, so that it's javascript does not interact with the notebook's. This also enables to create an iframe with the same size as the original matplotlib figure.

jwass commented 9 years ago

Hi @BibMartin - sorry for the super long delay.

I'm just curious about your environment - when you have cross domain issues loading the leaflet source, are there similar issues downloading map tiles? Are those served from a local server?

Definitely like the idea of allowing the source/css to be embedded in the page, though it should probably be done more explicitly through the template as you mentioned.

BibMartin commented 9 years ago

Hi @jwass - thanks for your answer

The environment I refer to is a Jupyter server, served on HTTPS through a reverse proxy based on nginx. Combined with the fact that I have to use firefox for some other external reasons...

One month later, I understand that this is mainly due to Firefox that don't like having an HTTP javascript loaded by an HTTPS web page. This is specific to javascript ; no problem with tiles or CSS. I will try to adapt this PR in the coming days :

BibMartin commented 9 years ago

Done.

jwass commented 9 years ago

@BibMartin Just made a few comments. I think if those are resolved we'll get this merged ASAP. Sorry again for the delay.

BibMartin commented 9 years ago

@jwass

The .replace() call can be removed now that you've included it explicitly in the template, right?

I agree. Changed in https://github.com/BibMartin/mplleaflet/commit/a528120b1365846c1ccec6e30192a4237cd24b57.

I couldn't get it working in the IPython notebook unless iframe was set to true. Otherwise we'd need the ipynb.html template. Is there any reason not to force iframe when calling display for the notebook here?

No. I was just willing not to be too intrusive in the existing code.

If not, probably makes sense to just remove the iframe argument and make that behavior the only part of the flow.

I agree. Changed in https://github.com/BibMartin/mplleaflet/commit/a528120b1365846c1ccec6e30192a4237cd24b57.

Please tell me if this is fine to you ; and if you want I squash the code before merge.

jwass commented 9 years ago

Looks great. Squash and then I'll merge. Thanks!!

BibMartin commented 9 years ago

@jwass Done

jwass commented 9 years ago

Thanks!!