Add Fast marker cluster to plugins #585
Conversation
There was a problem hiding this comment.
@JamesGardiner can you please add an entry in the change log and a Jupyter notebook demoing the new class? (Maybe you can just add an extra example in one of our current notebooks.)
| m._repr_html_() | ||
|
|
||
| out = m._parent.render() | ||
| print(out) |
| })(); | ||
| {% endmacro %}""") | ||
|
|
||
| def create_marker(self): |
There was a problem hiding this comment.
You probably do not need to do this as a function.
| @@ -19,6 +20,7 @@ | |||
|
|
|||
| __all__ = [ | |||
| 'MarkerCluster', | |||
| 'FastMarkerCluster', | |||
There was a problem hiding this comment.
How does this compares with our MarkerCluster besides speed? Can we replace the current one with this?
There was a problem hiding this comment.
There's some significant differences - the main one being that the FastMarkerCluster object calls super(FastMarkerCluster, self).__init__([]) - so no children are added and full functionality isn't available (for instance, calling get_bounds() returns [[None, None], [None, None]]`. I'd recommend keeping both and caveating FastMarkerCluster as being quick to draw 000's of points but loses functionality.
There was a problem hiding this comment.
I did not read the docstrings yet, but that information should be there.
|
@JamesGardiner I am a little bit swamped right now to review this. But this change is very welcomed! |
|
No problem @ocefpaf - I completely understand. I'm finding it difficult to get time to review the notebook failure in any case. Hopefully I can get all tests passing this weekend. |
| @@ -1,6 +1,7 @@ | |||
| 0.3.0 | |||
| ~~~~~ | |||
|
|
|||
| - Added `FastMarkerCluster` (James Gardiner #585 (proposed by @ruoyu0088)) | |||
| points far quicker than the MarkerCluster class. Be aware | ||
| that the FastMarkerCluster class does not retain a | ||
| reference to any marker data, and therefore methods such as | ||
| get_bounds() are not available when using it. |
There was a problem hiding this comment.
Can you expand on the difference from the "regular" MarkerCluster?
|
Ignore the Travis-CI coding standards failures as those are related to new and old notebooks. I will fix that later. Can you just comment on the docstring so we can merge this? Thanks for the effort and the patience! |
@JamesGardiner what is the status on this? I am planning on issuing a new release and it would be nice to get this PR merged before that. |
|
@ocefpaf - I can add the docstrings in before the end of this week if that helps? |
That works 😄 My original plan was to issue a new release before SciPy, but that is not going to happen. (Day job is keeping me busy.) |
… into fast-marker-cluster
folium/plugins/__init__.py
Outdated
| from .float_image import FloatImage | ||
| from .fullscreen import Fullscreen | ||
| from .heat_map import HeatMap | ||
| from .image_overlay import ImageOverlay | ||
| from .marker_cluster import MarkerCluster | ||
| from .polyline_text_path import PolyLineTextPath | ||
| from .fast_marker_cluster import FastMarkerCluster |
There was a problem hiding this comment.
F811 redefinition of unused 'FastMarkerCluster' from line 10
|
@JamesGardiner awesome! Thanks!! Just a minor comment and we are good to go. |
|
No problem, I've pushed a fix for the typo 👍 |
|
Awesome! Thanks!! |
…arker-cluster Add Fast marker cluster to plugins
This should close #416. Adds a faster way to add marker clusters to the output map, using a javascript based callback to construct MarkerCluster objects in the browser. Script originally proposed by @ruoyu0088. I've modified that original proposal to remove the dependency on flexx and tornado.
Users can specify their own callback string should they want to by passing it as the callback kwarg when creating a FasterMarkerCluster object.