randyzwitch / streamlit-folium

Streamlit Component for rendering Folium maps
https://folium.streamlit.app/
MIT License
468 stars 176 forks source link

Compatibility with folium 0.15.0 #148

Closed BastienGauthier closed 10 months ago

BastienGauthier commented 11 months ago

I upgraded to folium 0.15.0, and the 2 maps I tested returned as grey, no crash or anything.

From the changelog, i did not identified what could be the issue : https://[raw.githubusercontent.com/python-visualization/folium/main/CHANGES.txt](https://raw.githubusercontent.com/python-visualization/folium/main/CHANGES.txt)

randyzwitch commented 11 months ago

My guess would be this does something we don’t expect

  • Change internally where layers are added to the map (@Conengmo #1690)
Conengmo commented 11 months ago

Might very well be the case: https://github.com/python-visualization/folium/pull/1690. I don't know much about Streamlit, but let me know if I can help on the Folium side!

Conengmo commented 11 months ago

@BastienGauthier could you share something about the maps that caused the issue? And can you look at your browser console logs if there are any errors there?

Note that the Stamen tiles were deprecated, so if you are using those that might also cause issues.

BastienGauthier commented 11 months ago

@Conengmo , @randyzwitch : some precisions on the issue

No stamen tiles, you can actually test it with the simplest streamlit folium app:

from streamlit_folium import st_folium  
import folium  
maptest = folium.Map()  
st_data = st_folium(maptest)  

And you will get a grey map : image

Regarding the console, I am not sure what to look at, but here is an extract : image

(I am on Microsoft Edge Version 119.0.2151.44 - quite up to date, and same issue on Chrome)

Conengmo commented 11 months ago

Thanks Bastien.

I looked into it a little bit and it seems indeed related to the change to the Layer class. In Streamlit-Folium I suspect this line is contributing, where folium_map is a Map object, on which render() is called: https://github.com/randyzwitch/streamlit-folium/blob/960f6a40d39219f72b3d928dd9910ed12a3e60bd/streamlit_folium/__init__.py#L249

But in Folium layers are now added to the map through the parent Figure: https://github.com/python-visualization/folium/blob/84459ba089da13f2a699670a44fbe6e88e662e7b/folium/map.py#L69. It's basically an assumption in Folium that a Map is added to a Figure, and the Figure is being rendered. That's why often m.get_root().render() is used to get the full html representation.

Not sure what the full problem is and how it should be resolved. Maybe on the Folium side the element that adds a Layer to its parent should be a separate MacroElement child class.

BastienGauthier commented 11 months ago

For information, I just tested tu change the folium_map.render() into folium_map.get_root().render(), but I did not resolved magically this issue.

@blackary, maybe you have a better view on what could we test here to solve this ?

blackary commented 11 months ago

@BastienGauthier I tried the same, and found the same result. In theory, we just need a consistent way to get the script out that will render the map with all the objects on it. I will get some time to play with this eventually, but if anyone else can find a solution, that would be excellent.

BastienGauthier commented 11 months ago

I am kind of out of my league here so I will let you have a look, but I guess the failed test results may be of help, comparing the 0.15 results with the 0.14 assertions : https://github.com/randyzwitch/streamlit-folium/actions/runs/6813281431/job/18527487502

BastienGauthier commented 11 months ago

I did some investigation and created a small exemple to complete the issue.

import streamlit as st
from streamlit_folium import st_folium

import folium

m = folium.Map(location=[46.9, 3], zoom_start=6, prefer_canvas=True)

coucou_group = folium.FeatureGroup(name = "test", show=True).add_to(m)

popup_list = [
    [(46.9, 3), 40],
    [(47.9, 3), 40],
    [(49, 3.5), 30],
    [(46.2, 2), 50],
    ]

for i, popup in enumerate(popup_list):
    circle = folium.Circle(popup[0],radius=popup[1],popup=f"{i:03d}")
    circle.add_to(coucou_group)

Here are the results with v0.14 and v0.15 folium : test_v14.txt test_v15.txt

The only missing part is therefore the ".addTo(map_div)" missing everywhere, which is consistant with the explainations hereabove.

@Conengmo, do you think we could use some magic as for https://github.com/python-visualization/folium/blob/84459ba089da13f2a699670a44fbe6e88e662e7b/folium/map.py#L58C9-L58C26 here ? I didn't succeed to make this work, but do you think this way is a good solution ?

egelados commented 11 months ago

Seems that folium.raster_layers.ImageOverlay show parameter is ignored. This is why the maps turn out grey. They can only be displayed if LayerControl is enabled from the upper right corner widget.

Went back to folium==0.14.0 and streamlit-folium==0.15.0 and it works correctly.

BastienGauthier commented 11 months ago

Seems that folium.raster_layers.ImageOverlay show parameter is ignored. This is why the maps turn out grey. They can only be displayed if LayerControl is enabled from the upper right corner widget.

Went back to folium==0.14.0 and streamlit-folium==0.15.0 and it works correctly.

@egelados : Not sure to understand what you found here : the show parameter should be handled by the Layer class heritage in the folium.raster_layers.ImageOverlay. The point is to make streamlit-folium work with folium==0.15.0.

BastienGauthier commented 11 months ago

Also, folium_static (i.e. the one-way html component) is working : only the direct reading of the _template in _generate_leaflet_string of st_folium seems the issue. An equivalent of folium.Layer._add_layer_to_map sould be used I guess (I tried without success up to now)

BastienGauthier commented 10 months ago

Hello @giswqs, you could be of help here ! Did you succeed to correct this folium compatibility in your package ? If the answer is yes, do you think the solution is applicable here ?

Conengmo commented 10 months ago

I've been looking into a fix and I think I got something. @BastienGauthier do you have time to validate this fix? https://github.com/python-visualization/folium/pull/1834 Much appreciated.

BastienGauthier commented 10 months ago

I am not avaiblable now but I will look at this next week, thanks for the fix !

BastienGauthier commented 10 months ago

I tested it and its works like a charm ! Thanks @Conengmo ! If you can release a 0.15.1 with this fix, it will be much appreciated !

Conengmo commented 10 months ago

Great, thanks for checking! Next steps are making sure tests pass, merge and then do a v0.15.1 release indeed.

randyzwitch commented 10 months ago

Awesome, thanks for working with us to find the fix!

Once this makes it into folium, we’ll make it work on our side (cc @blackary )

patrontheo commented 10 months ago

Hey,

I think it has made it into folium 0.15.1 :) https://github.com/python-visualization/folium/releases

Thank you for your work guys !

randyzwitch commented 10 months ago

v0.17.0 to PyPI once release builds, conda later on