Update Map zoom_control variable to also allow string to set position#1884
Conversation
Conengmo
left a comment
There was a problem hiding this comment.
Implementation is good! I added two comments about the four possible string values. I'd like to make sure users won't make mistakes in these. I hope you can address these comments, afterwards this is good to go!
folium/folium.py
Outdated
| zoom_control : bool, default True | ||
| Display zoom controls on the map. | ||
| zoom_control : bool or position string, default True | ||
| Display zoom controls on the map, eventually specifying its position. |
There was a problem hiding this comment.
| Display zoom controls on the map, eventually specifying its position. | |
| Display zoom controls on the map. The default `True` places it in the top left corner. | |
| Other options are 'topleft', 'topright', 'bottomleft' or 'bottomright'. |
| # Zoom control position specified ? | ||
| if isinstance(zoom_control, str): | ||
| self.zoom_control_position = True | ||
| self.zoom_control = zoom_control |
There was a problem hiding this comment.
maybe useful to add a check here for valid values? Now if a users makes an error, they just get a grey screen. For example something like:
| self.zoom_control = zoom_control | |
| self.zoom_control = zoom_control | |
| if zoom_control not in {'topleft', 'topright', 'bottomleft', 'bottomright'}: | |
| raise ValueError("Incorrect value for `zoom_control`, choose from 'topleft', 'topright', 'bottomleft' or 'bottomright'.") |
There was a problem hiding this comment.
I had considered introducing a check on valid position values but looking at various plugins I could not find such a check so I thought it was considered as unnecessary ...
There was a problem hiding this comment.
I understand, this code base grew quite organically over time, so it's not always consistent. Thanks for putting it in!
|
Somethings going on with the tests, it's probably not related to this PR. |
|
Thanks for your contribution @berrfred! |
This is referred to issue #1865