Skip to content

Fix WMS only layer bug #274

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

Merged
merged 1 commit into from
Dec 6, 2015
Merged

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Dec 2, 2015

I guess that the overlay property is not always present.

@BibMartin tiny one to relax after the #266 rebase 😜

PS: The LayerControl seems to be swamping base_layers and overlays. I will take a look into that.

@BibMartin
Copy link
Contributor

The code is ok for me : it can only be better.
But I do you have cases when overlay property is not present ?

@ocefpaf
Copy link
Member Author

ocefpaf commented Dec 2, 2015

@ocefpaf
Copy link
Member Author

ocefpaf commented Dec 2, 2015

But I do you have cases when overlay property is not present ?

Reading the code closely I think that I am either doing something wrong or the bug is somewhere else.

When I do,

m = folium.Map([40,-100], zoom_start=4)

url = 'http://hfrnet.ucsd.edu/thredds/wms/HFRNet/USEGC/6km/hourly/RTV'

w = folium.WmsTileLayer(url,
                        name='HF Radar',
                        format='image/png',
                        layers='surface_sea_water_velocity',
                        attribution='HFRNet',
                        transparent=True)

w.add_to(m)
m.add_children(folium.LayerControl())
m

the WMS layer should be added as an overlay, but it is being added as a base_layer instead of the underlying openstreetmap layer.

overlay

@BibMartin any ideas?

@ocefpaf
Copy link
Member Author

ocefpaf commented Dec 2, 2015

I think I am starting to get your latest changes. (I am slow 😁)

The latest commit adds a missing overlay=True to the WMS layer. Let me know if you think we should get rid of the hasattr(val, 'overlay') or if we should leave it here.

PS: The openstreetmaps is still being added as an overlay instead of the base_layer. I will take a look and fix this in another PR though.

@ocefpaf
Copy link
Member Author

ocefpaf commented Dec 3, 2015

@BibMartin Let me know if this is OK now. I want to take another look at the layers control (making the initial tiles the base_layer and disabling the overlay by default), but I will send those in another PR to avoid confusion.

Update: rebased and good to go.

@ocefpaf ocefpaf force-pushed the wms_bug branch 2 times, most recently from 288644b to 37c72ef Compare December 3, 2015 15:25
@ocefpaf ocefpaf added this to the v0.2.0 milestone Dec 3, 2015
@ocefpaf ocefpaf added the bug An issue describing unexpected or malicious behaviour label Dec 3, 2015
@ocefpaf ocefpaf self-assigned this Dec 3, 2015
@BibMartin
Copy link
Contributor

@ocefpaf
It's okay for me.
Squash and go ?!

@ocefpaf
Copy link
Member Author

ocefpaf commented Dec 5, 2015

Done!

BibMartin added a commit that referenced this pull request Dec 6, 2015
Fix WMS only layer bug ; thanks @ocefpaf
@BibMartin BibMartin merged commit 1bfd240 into python-visualization:master Dec 6, 2015
@ocefpaf ocefpaf deleted the wms_bug branch December 6, 2015 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue describing unexpected or malicious behaviour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants