bump to leaflet 1.5.1 #1149
bump to leaflet 1.5.1 #1149
Conversation
|
The Following notebooks are broken with leaflet 1.5.0 and probably need some sort of update/fix:
|
|
Hi @ocefpaf, nice that you made a PR for this. I saw they released a hotfix, v1.5.1, can you update? https://leafletjs.com/2019/05/08/leaflet-1.5.1.html I don't know if this fixes the timesliderchoropleth notebook. I'll have a look at the dualmap plugin. |
| @@ -287,7 +287,7 @@ class CircleMarker(Marker): | |||
| Radius of the circle marker, in pixels. | |||
| **kwargs | |||
| Other valid (possibly inherited) options. See: | |||
| https://leafletjs.com/reference-1.4.0.html#circlemarker | |||
| https://leafletjs.com/reference-1.5.1.html#circlemarker | |||
There was a problem hiding this comment.
@Conengmo looks like this URL re-directs to 1.5.0 and makes me think that they are not going to publish a 1.5.1 version of the docs. I guess that we should make the sphinx link check a soft fail so we can merge this and create our docs. What do you think?
There was a problem hiding this comment.
You're right, it seems they always only publish docs for the major and minor versions. I don't know that much about Sphinx. I'm thinking now it may be easiest to only link to their documentation of the minor versions as well? So change all documentation links to 'reference-1.5.0'. Those links will also perhaps resolve faster since there's no redirect.
There was a problem hiding this comment.
It is properly redirected so I just moved the linkcheck part of sphinx to a "allow failures" tests. It was meant to be just a report and not really a hard error.
|
The TimeSliderChoropleth notebook runs without errors, but the time slider functionality doesn't work properly. It's not an external plugin, but a custom template using D3. I don't feel like learning d3 to be able to maintain the plugin, so maybe it's better to just deprecate the plugin. |
Let's do that in another PR. I'll fix the merge conflicts here in a moment. |
|
@Conengmo this is ready for another pass. |
|
I found one outdated link and pushed that to your branch. Other than that this looks good to go! I'm thinking though, maybe we should first make a 0.9.1 release with the fix for the geojson feature identifier. Then merge this afterwards for a new minor version release. What do you think? |
Oops. Missed that one.
Sure. Let me work on that releases. |
Done! PS: there is a transient failure in one of the notebooks but it is safe to merge. |
|
Thanks @ocefpaf! I'll go ahead and merge this one.
Awesome, let me know if I can help with anything.
That makes a lot of sense. It would be nice if we would have a single site with our readme, docs, examples. Everything under a URL that doesn't change with versions or source tree layout.
I don't know if we should use any as tests. I'd rather write explicit tests for the code and the browser functionality, with something like Selenium. Then again, if a notebook fails after a change in folium that is a good indicator something is wrong :) |
That is definitely better, the notebooks are kind of a poor's man browser tests. |
Closes #1147