Skip to content

Untoggle overlays and additional base layers#772

Merged
ocefpaf merged 10 commits intopython-visualization:masterfrom
Conengmo:hide-overlays
Jan 23, 2018
Merged

Untoggle overlays and additional base layers#772
ocefpaf merged 10 commits intopython-visualization:masterfrom
Conengmo:hide-overlays

Conversation

@Conengmo
Copy link
Copy Markdown
Member

@Conengmo Conengmo commented Nov 22, 2017

I worked on solving issue #606 and #571. I added functionality in the LayerControl class to untoggle overlays that have hidden=True. While I was there I simplified the checks in render(). If an item is an instance of Layer it will always have the overlay and control attributes.

I added the hide parameter to some classes to test it out, but not to all children of Layer yet.

What do you guys think, is this a good way to do this?

@Conengmo Conengmo changed the title WIP Hide overlays WIP Hide overlays (issue #606) Nov 22, 2017
self._parent._children.items() if isinstance(val, Layer)
and val.overlay and val.control])
self.overlays_hidden = [val.get_name() for val in
self._parent._children.values() if isinstance(val, Layer)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E128 continuation line under-indented for visual indent

folium/map.py Outdated
and val.overlay and val.control])
self.overlays_hidden = [val.get_name() for val in
self._parent._children.values() if isinstance(val, Layer)
and val.overlay and val.control and val.hide]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E128 continuation line under-indented for visual indent

@Conengmo Conengmo changed the title WIP Hide overlays (issue #606) WIP Hide overlays Nov 22, 2017
@ocefpaf
Copy link
Copy Markdown
Member

ocefpaf commented Nov 25, 2017

What do you guys think, is this a good way to do this?

Yep. That fits folium perfectly. Thanks!

Can you:

  • add the keyword to everything else that inherits from Layer;
  • change the nome from hide to... Maybe toggle? I accept other suggestions but hide seems like something that won't be displayed ever;
  • add an entry to the changelog/

Frank added 4 commits November 28, 2017 17:04
`show` toggles whether the layer is shown on opening. It is True by default.

Also did some small refactoring here and there to make __init__'s and docstrings more consistent.
Doesn't really make sense to inherit from TileLayer. So I made it a bit more consistent by inheriting directly from Layer.
@Conengmo
Copy link
Copy Markdown
Member Author

I added the show parameter to all classes that inherit from Layer. At least, I hope I got them all :)
show seemed a good choice, I saw it's also being used in #778.

I noticed Heatmap and HeatMapWithTime inherited from TileLayer, that didn't really make sense to me. They also didn't use anything from TileLayer. So for consistency I changed both classed to inherit from Layer.

I also did some work on the use of name, overlay and control in __init__ statements and docstrings in files I touched. It should be fairly consistent now.

I didn't test all code I touched. I did test creating some maps with GeoJson objects, with Featuregroups, MarkerClusters and HeatMaps, and everything seemed to work alright.

Let me know if there's something you want different or added.

@Conengmo Conengmo changed the title WIP Hide overlays Hide overlays Nov 28, 2017
@Conengmo
Copy link
Copy Markdown
Member Author

Conengmo commented Dec 7, 2017

I noticed that according to Leaflet, any additional base layers should not be added to the map. In Folium all base layers are added to the map. As a result the last base layer is selected in layer control, while the first base layer is displayed.

Fixing this by not adding additional base layers to the map is hard, since we then need to check this in every class that inherits from Layer. Removing the additional base layers in LayerControl also works, so I added it here.

@Conengmo Conengmo changed the title Hide overlays Untoggle overlays and additional base layers Dec 7, 2017
@ocefpaf ocefpaf merged commit 9e2ee74 into python-visualization:master Jan 23, 2018
@ocefpaf
Copy link
Copy Markdown
Member

ocefpaf commented Jan 23, 2018

Perfect! Thanks @Conengmo!!

@carlospendola
Copy link
Copy Markdown

thanks master! @Conengmo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants