-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Use firefox headless instead of phantomjs #1015
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
Use firefox headless instead of phantomjs #1015
Conversation
folium/folium.py
Outdated
import time | ||
import warnings | ||
|
||
from branca.colormap import StepColormap |
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.
This is a bad rebase :-/ Let me fix those.
94a7f4f
to
b1ffec1
Compare
b1ffec1
to
2e3470f
Compare
@Conengmo rebased and ready for a review. This one is not too important b/c we don't really advertise this functionality yet. |
Looks good, and the tests work fine on Travis. Only problem is that the temporary file doesn't work on Windows. We had this problem before: I'll see if I can find a solution, otherwise we'll merge it as is. |
I can get the tempfile stuff to work if instead of using
Note that this returns the file path, not a file descriptor. What do you think @ocefpaf? |
I'm fine with that solution. (Actually I used something similar in the past when writing netCDF files, my bad for ignoring Windows here.) |
@Conengmo I'll take a look at the failures later but it seems that only Python 2.7 is affected and, b/c this feature is not really ready for prime time, I'm OK making it Python 3 only. (We already agreed that the next release is the last one for Python 2.7 anyway.) |
def m_png(): | ||
yield folium.Map(png_enabled=True) | ||
|
||
def test__repr_html_is_str(m): |
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.
E302 expected 2 blank lines, found 1
Spoke too soon. All the failures were related to new |
Merging this to b/c of the Travis-CI fixes in the last commit are needed for a new release. |
Closes #891
I chose firefox b/c it is easier to test on Travis-CI and we found some rendering issues with chrome/chromium in the past.
PS: @Conengmo the
_png()
is not a first class citizen yet. Not sure what else we could do to make it more robust. Maybe a driver selection option?