Make Folium follow the Leaflet class hierarchy more closely

[hansthen]

Is your feature request related to a problem? Please describe. I find I sometimes need to patch or extend Folium classes. This is to access Leaflet functionality that would be accessible directly from javascript. Starting as a Leaflet user before moving to Folium I find this annoying at times.

Describe the solution you'd like The functionality I miss resides in a few classes in the Leaflet class hierarchy. Notably:

1. L.Class
2. L.Evented
3. L.Control

My proposal would be to implement these classes in Folium.

folium.Class will provide the include method, which makes it really easy to customize any Leaflet object.
folium.Evented provides the possibility to add javascript event handlers using on(event=JsCode(). folium.Control factors out the common code for controls.

folium.Class will be a subclass of branca.MacroElement. It will be the parent class for all Leaflet based classes in Folium. folium.Evented would become a superclass of folium.Layer.
folium.Control will become the superclass for the core folium controls and also for the plugins that now derive from MacroElement and implement a leaflet Conrol.

These changes should be backwards compatible for most uses.

There is already a PR for adding an event handler and for a Leaflet.Control object, https://github.com/python-visualization/folium/pull/1903 and https://github.com/python-visualization/folium/pull/1902. If you agree this proposal is a good idea, I will rewrite those PRs, to make them align with the class hierarchy.

Describe alternatives you've considered So far I have been copying and modifying Folium classes to get what I want.

Implementation I am willing to implement this.

[Conengmo]

Interesting idea! I'd love to discuss this further. I'll give some thoughts, counter arguments really, and I hope we can talk about it.

I don't think following the Leaflet class hierarchy in itself should be a goal, my question would be what benefit it would bring. So where it brings us something, probably mostly less duplicate code, then it makes sense. But Python is different from Javascript, plus we have an existing code base to deal with, so it may be we choose to make different abstractions than Leaflet has.

The class hierarchy in Branca + Folium is already pretty complex, and even I with how much experience I have with these projects sometimes have trouble grokking how things inherit. Or maybe that's just me? ;) I'd opt for a less complex class structure if possible. I think this is a common issue in software, see 'multiple inheritance'.

One radical thought, also applies to https://github.com/python-visualization/folium/issues/1942: we're not constrained to Folium. We can make a fork (New Folium?) and start over, build it just the way you envision it. If that makes things much better in terms of features or maintainability, that's great! We can port things back, or just point people to the new version.

Some general thoughts, hope you find it interesting.

[hansthen]

Thanks for your reply! Your comments are really welcome. Counter arguments make for better decisions. I agree, following the Leaflet class hierarchy should not be goal in itself.

I also think some things can become clearer if we follow the Leaflet class hierarchy. I think there is a satisfying simplicity to the Leaflet class diagram. I find it very intuitive (ymmv). https://leafletjs.com/examples/extending/class-diagram.html.

As a new Folium user coming from Leaflet, I was confused sometimes by the Branca elements as well as the multiple inheritance. (What exactly does MacroElement do?)

My main goal with following the Leaflet class hierarchy more closely is not to reduce code, but to give end-users capabilities they would not have without extending Folium itself. By introducing Control, 'EventedandClass` I introduce the following three capabilities:

1. Control allows users to create a new control plugin in python without having to create a new plugin class.
2. Evented adds javascript event handlers to Layer (and Map).
3. Class allows users to override Leaflet class defaults, which gives powerful customization abilities.

My hope is that by giving users more power, we reduce the maintenance burden on ourselves.

In general I would not want to fork Folium. I really love the library as is. I would not want to compete with it.

I suggest the following: I work out the three different proposals in separate pull requests. You can then judge each proposal to see if it improves the clarity of the class hierarchy. Is that a good way forward?

[ocefpaf]

In general I would not want to fork Folium. I really love the library as is. I would not want to compete with it.

And you both are authors/owners of folium now 😜, no need to fork unless you have a plan to sunset it.

[Conengmo]

I suggest the following: I work out the three different proposals in separate pull requests. You can then judge each proposal to see if it improves the clarity of the class hierarchy. Is that a good way forward?

Sure, let's see the ideas! Maybe when you make PR's to save yourself some time, don't apply it on everything yet.