Add minimap plugin #968

ColinTalbert · 2018-09-21T16:24:42Z

I wanted a locator map and luckily found this plugin:https://github.com/Norkart/Leaflet-MiniMap

The realm of what is possible in leaflet is awesome!

Hopefully this pluggin can be useful to others. See it in action here: http://nbviewer.jupyter.org/github/talbertc-usgs/folium/blob/minimap/examples/MiniMap.ipynb

stickler-ci · 2018-09-21T16:24:52Z

folium/plugins/minimap.py

+        figure.header.add_child(JavascriptLink('https://cdnjs.cloudflare.com/ajax/libs/leaflet-minimap/3.6.1/Control.MiniMap.js'))  # noqa
+
+        figure.header.add_child(CssLink('https://cdnjs.cloudflare.com/ajax/libs/leaflet-minimap/3.6.1/Control.MiniMap.css'))  # noqa
+        figure.add_child(self.tile_layer)


W292 no newline at end of file

Conengmo

Great PR Colin! This fits folium very well and I'm excited to start using this myself. I have a few comments, hope you can look at them. Also look at the stickler comment about the missing newline.

Should be easy to fix. Then we're good to get merging, after adding a line to the changelog which we'll do at the very last.

Conengmo · 2018-09-28T11:01:06Z

folium/plugins/__init__.py

@@ -50,4 +51,5 @@
    'TimestampedGeoJson',
    'TimestampedWmsTileLayers',
    'Search',
+    'MiniMap'


Please add a comma

Conengmo · 2018-09-28T11:03:01Z

folium/plugins/minimap.py

+    toggle_display: bool, default
+        Sets whether the minimap should have a button to minimise it.
+
+    auto_toggle_display: bool, default


You're missing some default values here

Conengmo · 2018-09-28T11:03:09Z

folium/plugins/minimap.py

+    collapsed_height: int, default 25
+        The height of the toggle marker and the minimap when collapsed
+
+    zoom_level_offset: int, defalut -5


Conengmo · 2018-09-28T11:03:42Z

folium/plugins/minimap.py

+    Parameters
+    ----------
+    tile_layer: folium tilelayer, str (Optional)
+


No need to have empty lines between the parameters

I'm still seeing them on the 'files changed' page, am I missing something?

Conengmo · 2018-09-28T11:06:21Z

folium/plugins/minimap.py

+
+    Parameters
+    ----------
+    tile_layer: folium tilelayer, str (Optional)


This could use a line with explanation. e.g.:

tile_layer: folium TileLayer object or str, optional Provide a folium TileLayer object or the wanted tiles as string. If not provided it will use the default of `TileLayer`.

Conengmo · 2018-09-28T11:08:53Z

tests/plugins/test_minimap.py

+# -*- coding: utf-8 -*-
+
+"""
+Test FloatImage


Wrong title.

Conengmo · 2018-09-28T11:11:57Z

folium/plugins/minimap.py

+        Set it to any valid zoom level, if unset zoomLevelOffset is used instead.
+
+    center_fixed: bool, default False
+        Applies a fixed position to the minimap regardless of the main map's view / position. P


Word is broken

Conengmo · 2018-09-29T10:19:55Z

One more thing, could you also add a single entry to the plugins notebook? The aim is to have an overview of all available plugins.

ColinTalbert · 2018-10-12T23:04:56Z

Let me know if you see anything else!

Conengmo

Thanks for the changes! We're almost there. I'm still seeing some empty lines. Also I have a question about a line I think may be redundant.

By the way, the Travis build failure is unrelated to your edits, so after your next commit (or if we restart it) it should pass.

Conengmo · 2018-10-13T07:48:56Z

folium/plugins/minimap.py

+    Parameters
+    ----------
+    tile_layer: folium tilelayer, str (Optional)
+


I'm still seeing them on the 'files changed' page, am I missing something?

Conengmo · 2018-10-13T07:49:59Z

folium/plugins/minimap.py

+
+    def __init__(self, tile_layer=None, position='bottomright', width=150,
+                 height=150, collapsed_width=25, collapsed_height=25,
+


Empty line in between the function arguments

Regarding the optional requirement above, It defaults to to whatever TileLayer defaults to (OSM I think) but I'm not sure I should hard code this in. So I put this in which might have been missing when you looked:
https://github.com/talbertc-usgs/folium/blob/minimap/folium/plugins/minimap.py#L23

Conengmo · 2018-10-13T07:50:30Z

folium/plugins/minimap.py

+        figure.header.add_child(JavascriptLink('https://cdnjs.cloudflare.com/ajax/libs/leaflet-minimap/3.6.1/Control.MiniMap.js'))  # noqa
+
+        figure.header.add_child(CssLink('https://cdnjs.cloudflare.com/ajax/libs/leaflet-minimap/3.6.1/Control.MiniMap.css'))  # noqa
+        figure.add_child(self.tile_layer)


Is adding self.tile_layer to figure needed? It seems the tile layer is already in your template. Can you try if this line is indeed redundant?

Good catch. I'm still getting comfortable with the template usage. I've removed this line.

Conengmo · 2018-10-15T19:52:22Z

@talbertc-usgs I made some small changes to finish this PR and merge it, but I'm not allowed to push to your branch. Can you either allow access for folium maintainers to your branch, merge my branch into yours, or copy over the changes? After that I'll merge this.

stickler-ci · 2018-10-15T20:44:23Z

folium/folium.py

@@ -8,6 +8,7 @@
 from __future__ import (absolute_import, division, print_function)

 import os
+import tempfile


F401 'tempfile' imported but unused

I merged in your changes but I'm not sure where this stickler issue came from.

It seems git got a bit screwed up. I'm no git wizard so I can't tell exactly what went wrong. I did a rebase and it seems my branch is now okay. Can you try to merge again, or otherwise rebase as well?

Conengmo · 2018-10-15T21:16:50Z

Looking good! Nice you got the git thing solved. I'll wait for the travis check to finish and merge this tomorrow.

ColinTalbert · 2018-10-15T21:21:39Z

Thanks, I'm no git expert either. I can tread water to a certain point, but then I flail around quite a bit.

…

-- Colin Colin Talbert Data Manager, Analyst and Developer US Geological Survey, Fort Collins Science Center 2150 Centre Ave. Bldg. C Fort Collins, CO 80526 (970) 226-9425 http://orcid.org/0000-0002-9505-1876 Work schedule: Monday, Thursday - 7:00 - 5:00 Tuesday - Wednesday - 7:00 - 3:10 Friday (Telework) - 7:00 - 5:00

On Mon, Oct 15, 2018 at 3:17 PM Frank ***@***.***> wrote: Looking good! Nice you got the git thing solved. I'll wait for the travis check to finish and merge this tomorrow. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#968 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AWHgav86Ig0-QsAbb7cyt-uBha7c0qaQks5ulPtMgaJpZM4W0fCU> .

Conengmo · 2018-10-16T08:42:57Z

Merged! Congrats and thanks for your efforts @talbertc-usgs. We'll be looking at your other PR after this.

ColinTalbert · 2018-10-16T15:01:08Z

Thanks, and thanks for your work on this project. The second one's not quite as straightforward so let me know.

…

-- Colin Colin Talbert Data Manager, Analyst and Developer US Geological Survey, Fort Collins Science Center 2150 Centre Ave. Bldg. C Fort Collins, CO 80526 (970) 226-9425 http://orcid.org/0000-0002-9505-1876 Work schedule: Monday, Thursday - 7:00 - 5:00 Tuesday - Wednesday - 7:00 - 3:10 Friday (Telework) - 7:00 - 5:00

On Tue, Oct 16, 2018 at 2:43 AM Frank ***@***.***> wrote: Merged! Congrats and thanks for your efforts @talbertc-usgs <https://github.com/talbertc-usgs>. We'll be looking at your other PR after this. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#968 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AWHgahYo1VsK9KUrSxXx1CT9guUPxWEfks5ulZwbgaJpZM4W0fCU> .

ocefpaf · 2018-10-16T15:58:28Z

Thanks @talbertc-usgs! Awesome PR!! And thanks for @Conengmo for the nice review. This is a nice addition to folium 🎉

Add inital minimap plugin

3191c20

stickler-ci reviewed Sep 21, 2018

View reviewed changes

ColinTalbert added 2 commits September 21, 2018 10:25

correction for stickler-ci error.

604f9b5

Add the correct demo notebook

ea11406

ColinTalbert mentioned this pull request Sep 21, 2018

Minimap pluggin #969

Closed

Conengmo requested changes Sep 28, 2018

View reviewed changes

Conengmo changed the title ~~Add inital minimap plugin~~ Add minimap plugin Sep 28, 2018

ColinTalbert force-pushed the minimap branch from 2e3a79e to 25e2a29 Compare October 12, 2018 23:01

Addressing PR comments.

34010e5

ColinTalbert force-pushed the minimap branch from 25e2a29 to 34010e5 Compare October 12, 2018 23:04

Conengmo reviewed Oct 13, 2018

View reviewed changes

ColinTalbert and others added 3 commits October 15, 2018 08:51

Addressing PR review comments v2

4453b23

Docstring formatting

e3f52fa

Entry in changelog

d13fa15

Conengmo added the waiting for changes This PR has been reviewed and changes are needed before merging label Oct 15, 2018

stickler-ci reviewed Oct 15, 2018

View reviewed changes

ColinTalbert and others added 7 commits October 15, 2018 23:04

Add inital minimap plugin

3e9ac11

correction for stickler-ci error.

9a2cebb

Add the correct demo notebook

3cd3aae

Addressing PR comments.

4d644e3

Addressing PR review comments v2

95c53fd

Entry in changelog

1ad0116

Again apply docstring formatting

dc97cde

Conengmo approved these changes Oct 15, 2018

View reviewed changes

Conengmo removed the waiting for changes This PR has been reviewed and changes are needed before merging label Oct 15, 2018

Conengmo merged commit 584ea34 into python-visualization:master Oct 16, 2018


		def __init__(self, tile_layer=None, position='bottomright', width=150,
		height=150, collapsed_width=25, collapsed_height=25,

Add minimap plugin #968

Add minimap plugin #968

Conversation

ColinTalbert commented Sep 21, 2018

Choose a reason for hiding this comment

Conengmo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Conengmo Sep 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Conengmo commented Sep 29, 2018

ColinTalbert commented Oct 12, 2018

Conengmo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Conengmo Oct 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Conengmo commented Oct 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Conengmo Oct 15, 2018 • edited Loading

Choose a reason for hiding this comment

Conengmo commented Oct 15, 2018

ColinTalbert commented Oct 15, 2018 via email

Conengmo commented Oct 16, 2018

ColinTalbert commented Oct 16, 2018 via email

ocefpaf commented Oct 16, 2018

Conengmo Sep 28, 2018 •

edited

Loading

Conengmo Oct 13, 2018 •

edited

Loading

Conengmo Oct 15, 2018 •

edited

Loading