multiple markers with one icon #2053
Conversation
| self.marker = marker | ||
| self.icon = icon | ||
|
|
||
| def register_marker(self, marker: "Marker") -> None: |
There was a problem hiding this comment.
To me it would feel more natural to have a setIcon method on Marker (which generates the same code).
| if icon is not None: | ||
| # here we make sure the icon is rendered after the last marker that needs it |
There was a problem hiding this comment.
It feels more natural to make SetIcon a child of Marker and add the Icon to the map instead of to the Marker.
| if icon is not None: | ||
| # here we make sure the icon is rendered after the last marker that needs it | ||
| if icon._parent is not None: |
There was a problem hiding this comment.
These two lines feel a bit awkward. Also they are not necessary if we make Icon a child of Map like in my earlier comments.
| if icon is not None: | ||
| # here we make sure the icon is rendered after the last marker that needs it | ||
| if icon._parent is not None: | ||
| del icon._parent._children[icon.get_name()] | ||
| self.add_child(icon) |
There was a problem hiding this comment.
In my proposal, the Icon could be added to the map (or to the parent of the Marker). To ensure backwards compatibility we can add the icon from the marker. We'd have to move it either to add_to(m) or to the render() method from the marker.
I am not sure if add_child is idempotent. At least for Folium it can do no harm to make add_child ensure that children are added only once. Otherwise, we'd have to ensure the child is added only once from here.
| if icon is not None: | ||
| # here we make sure the icon is rendered after the last marker that needs it | ||
| if icon._parent is not None: | ||
| del icon._parent._children[icon.get_name()] | ||
| self.add_child(icon) | ||
| self.icon = icon |
There was a problem hiding this comment.
If you follow my setIcon proposal, we could call it here.
This is a great idea. I really like this proposal. It is very pragmatic and solves the immediate reported issue. In the review are a few suggestions, mainly to add the generated icon to the map, instead of the Marker. I think it will make the resulting code simpler. User code would be like this: However, in the review there are also suggestions to make it backwards compatible. m = Map((0, 0)) |
This is an exploration of an idea.
Allow using one Icon object for multiple markers by:
Test it out with:
We could extend this to all marker and icon like classes. This does not apply to popups and tooltips, since those are not exclusively used markers, which icons are.