Add layers in Layer

[Conengmo]

Our current approach to adding layers to the map and how that works with LayerControl is:

• each Layer subclass adds itself to its parent
• then when rendering LayerControl we remove the layers that have show=False.

This has two issues, one big and one minor:

• Performance. Layers are added, then removed, which is overhead. See Performance hit when FeatureGroup is created with show=False. #1573.
• Conceptually this doesn't correspond to how Leaflet is designed.

Address this by letting subclasses of Layer only add themselves to their parent if show=True.

We can remove the code from each of the subclasses and render it in Layer instead.

If you're asking, whoever did this in the first place? It was me, my first PR on this project :) #772. Then slight changes were made in LayerControl in #1101.

Downsides

Order of multiple base layers

The downside is that this changes behavior when you add multiple base layers. Currently only the first one is enabled. With this change the last one added will be enabled.

I'm conflicted whether this is something we should want to avoid. It does fit more closely to how Leaflet works. If you want to add multiple TileLayers, you will have to use the show parameter to define which one you want to see. For example:

m = Map(tiles='cartodbpositron')
TileLayer('stamen toner', show=False).add_to(m)

Downstream packages

Packages that use folium might suffer from this change, which I would call an internal breaking change. End-users shouldn't notice it, but others that develop using folium may. For example tests that test the correctness of renders will have to update.

folium convention

It may confuse folium developers that Layer subclasses don't add themselves to their parent, while other classes do.

Change parent class of two plugins

• SideBySideLayers currently inherits from Layer, but is not actually a layer-type class. Inherit from MacroElement instead.
• Same for TimestampedWmsTileLayers`, it's not a layer itself.

I'll probably split these two changes of into separate PRs.

[ocefpaf]

I'm OK with this change. It made sense in the past but getting closer to leaflet and snappier maps are worth the breaking changes. We should probably aim to a 1.0 release anyway. So, if you have more breaking changes, now is the time ;-p

[Conengmo]

Thank you for the review.

We should probably aim to a 1.0 release anyway.

Agreed. I'd like to include the improved documentation, other than that I don't have big ideas for 1.0.

If you have more breaking changes, now is the time

That's a good point. I'll think on it.

I did consider doing a refactor of our modules, especially map.py and features.py. But in the end I don't think it's worth the hassle.

[Conengmo]

@randyzwitch @giswqs since I know you both have projects that use folium, I wanted to give you a heads-up about this upcoming (breaking) change.

• If you have tests that use the 'raw' templates of Layer subclasses, you may need to make changes to those.
• It's not inconceivable this change introduces a bug in functionality, please let us know if that's the case.
• We will require the user to explicitly define what base maps they want to show. If you have code that adds base maps, you may need to add the show parameter.

Please let me know if you have questions or comments.

[giswqs]

Thanks for the heads up. I will keep an eye on this.