Added polyline offset plugin support with example data and script #614

fermumen · 2019-04-09T15:36:25Z

Pull Request

Hi,

I have implemented https://github.com/bbecquet/Leaflet.PolylineOffset which is great for visualizing directional road flows on road networks. I was wondering if this would be the best way to share the work with the community.

Essentially I modify the layers file and js sources to accept 'offset' as a variable and add new dependencies for the plug-in

Best regards,
Fernando

CLAassistant · 2019-10-02T15:53:39Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ fermumen
❌ Fernando Munoz Mendez

Fernando Munoz Mendez seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

trafficonese · 2020-01-22T09:50:40Z

inst/htmlwidgets/leaflet.yaml

+  - name: leaflet-polylineoffset
+    version: 1.1.1 # match leaflet version
+    src: "htmlwidgets/plugins/leaflet-polylineoffset"
+    stylesheet: leaflet.polylineoffset.js


It should be script instead of stylesheet. Otherwise the js-file would be added as css-link

Isnt the src in /lib instead of /plugin?

trafficonese · 2020-01-22T10:04:03Z

scripts/offset_example.R

@@ -0,0 +1,6 @@
+# example
+corunaroads <- readRDS("./data/corunaroads.rds")


I am not able to install your PR because of the .rds file. Could you either save it as .rda file or maybe just create some lines from sampled coordinates.

file 'corunaroads.rda' has magic number 'X'
Use of save versions prior to 2 is deprecated

Also I see that your example uses readRDS("./data/corunaroads.rds") but the PR has only data/corunaroads.rda?

I found out that the rda file was a badly renamed rds file hence the error. I've managed to fix it in the latest commit.

trafficonese · 2020-01-22T10:07:12Z

R/plugin-polylineoffset.R

+    htmltools::htmlDependency(
+      "leaflet-polylineoffset",
+      "1.1.1",
+      system.file("htmlwidgets/lib/leaflet-polylineoffset", package = "leafletfmm"),


It probably should be package = "leaflet"

trafficonese · 2020-01-22T10:18:16Z

R/layers.R

@@ -1128,6 +1128,7 @@ addPolylines <- function(
  fill = FALSE,
  fillColor = color,
  fillOpacity = 0.2,
+  offset = NULL,


I think new parameters should be added at the end to keep backward compatibility for positional arguments.

The documentation for offset is missing.

I've added the documentation for the parameter but I'm not sure what's the best way to update the man files. Is it just a case of using devtools::document() ?

yes, or you can change the settings in RStudio under Build > More > Configure Build Tools. In the Panel under "Build Tools" you can click the "Configure" Button and check "Install and Restart".

If you want to leave the dataset "corunaroads" in the PR, then you need to document that aswell to pass the R checks. You can see the documentation for the other datasets in /R/data.R. Then document & reinstall and resubmit the changes.

Then you should pass the Travis Checks for R. I'm not sure about the Javascript Check.

trafficonese

Thanks for your work. I was also very interested in using this plugin.

I couldn't get your PR to run since I am having problems reading the .rda/.rds file with R 3.6.1, because it was probably saved with version = 2 which is for R 1.4.0 to R 3.5.0.

So this call:
remotes::install_github("rstudio/leaflet#614")

resulted in this warning:

file 'corunaroads.rda' has magic number 'X'
Use of save versions prior to 2 is deprecated

Could you maybe use some sampled coordinates or even the data atlStorms2005? Thats a SpatialLinesDataFrame which is already included in the package.

…ependencies, added corunaroads as a valid rda and updated the example

fermumen · 2020-01-22T14:39:21Z

@trafficonese I've gone through your comments hopefully you'll be able to install it and test it now.

removed corunaroad dependency, and changed the example for it to use atlStorms2005

fermumen · 2020-01-22T15:40:11Z

I decided to do without corunaroads as it's probably a bit redundant. Somehow running document() has created a conflict on map-shiny.Rd. Any idea why?

trafficonese · 2020-01-22T15:49:02Z

I think it is because of the new roxygen version. But you should be able to look at the conflicts, resolve them and merge the results.

fermumen · 2020-01-22T17:02:59Z

@trafficonese The R checks are ok but as you said we are not passing the Javascript one. I'm a bit out of my depth in JS, unfortunately. It is entirely possible that I haven't properly followed these steps? I'm not sure since I prepared the pull request in April last year and I don't remember 😅

trafficonese · 2020-01-22T17:08:06Z

Unfortunately i'm no JS-Pro either and I dont know exactly what the checks are doing.
But did you use yarn build ? This should build a fresh /htmlwidgest/leaflet.js. Its not included in your PR so you probably skipped that part :)

schloerke · 2020-01-22T18:09:47Z

Yes. Calling yarn build in the terminal will be necessary for the JS check to pass.

fermumen · 2020-01-22T18:39:34Z

Success! Thanks @schloerke and @trafficonese, the fresh yarn build did the trick.

trafficonese · 2020-01-27T14:06:16Z

I have a little suggestion:

Could you include the leafletPolylineoffsetDependencies only if an offset is used?
Otherwise the dependency is always added, but not always needed/required.

So if offset is 0/NULL/NA, dont include the dependency.

fermumen · 2020-01-31T10:53:50Z

For some reason, I'm getting some error when installing, but I think is entirely due to my machine:
ERROR: dependency 'leaflet.providers' is not available for package 'leaflet'

I'll try to fix it but in the meanwhile, I haven't been able to install the package and test the new modifications.

Edit:
I installed the latest dev version of 'leaflet.providers' I'm not sure why it stopped working but I'm guessing it could be an issue with MRAN

fermumen · 2020-01-31T11:57:25Z

It looks like the Polylineoffset dependency is being added to the maps regardless we use leafletPolylineoffsetDependencies or not. Any idea why?

AntonWijbenga · 2021-12-13T10:38:13Z

Are issues with lines becoming circles or other erratic shapes solved in this version? The issues are discussed in the issue list of the bbecquet repository. For example here: bbecquet/Leaflet.PolylineOffset#1

I once adjusted a fork that had fixed such issues and made it compatible with the never version fo Leaflet (v2?). I use ShinyJS to include the .js source and then just use the offset parameter in the addPolylines function. Example: addPolylines(..., options=list(offset=3), ...) which works.

The JS file I use can be found here:
https://traphx.maptm.nl/leaflet.polylineoffset_arw.js

I have limited experience with package building and/or JavaScript. Otherwise I'd give it a go myself.

fermumen · 2021-12-13T11:13:29Z

Hi @AntonWijbenga,

Those issues with erratic shapes exist in the version implemented here, particularly the circles problem would happen for very complex shapes. In the past I have managed to mitigate it by simplifying the shape with rmapshaper package. I haven't touched this PR in ages (two years) but if there is a version of the polyline offset that solves it you should be able to include it in the appropriate folder in inst/htmlwodgets/lib/ to make it work.

ainefairbrother · 2024-02-15T20:48:44Z

Hi, thanks for implementing this feature! I would really like to use it, but can't find any documentation regarding how to implement this. Could you please provide a gist?

Thanks again, Aine

fermumen · 2024-03-25T11:20:04Z

Hi, thanks for implementing this feature! I would really like to use it, but can't find any documentation regarding how to implement this. Could you please provide a gist?

Thanks again, Aine

Hi!

This was never merged to the main branch, and it was implemented a while ago. Not sure how easy it would be to make it work but try remotes::install_github("rstudio/leaflet#614") and perhaps use a CRAN mirror for Early 2020, you can look at the example in scripts from this PR. Alternatively if you can switch to python, the Folium team has this implemented here

fermumen added 3 commits April 9, 2019 16:24

Added polyline offset plugin support with example data and script

a6d926e

Added polyline offset plugin support with example data and script

3b90463

changed extension

583640e

fermumen marked this pull request as ready for review April 10, 2019 08:21

trafficonese reviewed Jan 22, 2020

View reviewed changes

trafficonese suggested changes Jan 22, 2020

View reviewed changes

Fernando Munoz Mendez added 2 commits January 22, 2020 12:31

Fixed yaml with correct path and script instead of stylesheet

9c92534

moved offset parameter to the end, chaged package of polylineoffset d…

a741822

…ependencies, added corunaroads as a valid rda and updated the example

Created documentation for new parameter,

1aecc95

removed corunaroad dependency, and changed the example for it to use atlStorms2005

Merge branch 'master' into master

dade48a

refreshed leaflet.js by running YARN

818a308

Only using polyline offset dependencies when offset is defined

81ca4f1

Improved offset dependency logic.

0d0e745

trafficonese mentioned this pull request Feb 6, 2020

Polyline Offset trafficonese/leaflet.extras#170

Closed

ainefairbrother mentioned this pull request Feb 15, 2024

No documentation on the Polyline 'offset' functionality #901

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added polyline offset plugin support with example data and script #614

Added polyline offset plugin support with example data and script #614

fermumen commented Apr 9, 2019

CLAassistant commented Oct 2, 2019 •

edited

Loading

trafficonese Jan 22, 2020

trafficonese Jan 22, 2020

trafficonese Jan 22, 2020

fermumen Jan 22, 2020

trafficonese Jan 22, 2020

trafficonese Jan 22, 2020

trafficonese Jan 22, 2020

fermumen Jan 22, 2020

trafficonese Jan 22, 2020

trafficonese Jan 22, 2020

trafficonese left a comment

fermumen commented Jan 22, 2020

fermumen commented Jan 22, 2020

trafficonese commented Jan 22, 2020

fermumen commented Jan 22, 2020

trafficonese commented Jan 22, 2020

schloerke commented Jan 22, 2020

fermumen commented Jan 22, 2020

trafficonese commented Jan 27, 2020

fermumen commented Jan 31, 2020 •

edited

Loading

fermumen commented Jan 31, 2020

AntonWijbenga commented Dec 13, 2021

fermumen commented Dec 13, 2021

ainefairbrother commented Feb 15, 2024

fermumen commented Mar 25, 2024

		@@ -0,0 +1,6 @@
		# example
		corunaroads <- readRDS("./data/corunaroads.rds")

Added polyline offset plugin support with example data and script #614

Are you sure you want to change the base?

Added polyline offset plugin support with example data and script #614

Conversation

fermumen commented Apr 9, 2019

Pull Request

CLAassistant commented Oct 2, 2019 • 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

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

trafficonese left a comment

Choose a reason for hiding this comment

fermumen commented Jan 22, 2020

fermumen commented Jan 22, 2020

trafficonese commented Jan 22, 2020

fermumen commented Jan 22, 2020

trafficonese commented Jan 22, 2020

schloerke commented Jan 22, 2020

fermumen commented Jan 22, 2020

trafficonese commented Jan 27, 2020

fermumen commented Jan 31, 2020 • edited Loading

fermumen commented Jan 31, 2020

AntonWijbenga commented Dec 13, 2021

fermumen commented Dec 13, 2021

ainefairbrother commented Feb 15, 2024

fermumen commented Mar 25, 2024

CLAassistant commented Oct 2, 2019 •

edited

Loading

fermumen commented Jan 31, 2020 •

edited

Loading