23 new function aggregate lineparts sfr

[SanderDevisscher]

fixes #23

een functie die stukjes van een lijn aan elkaar verbind

[soriadelva]

The function works, however, I have a few changes that would need to be adressed:

• Fix the notes that pop up after running devtools::check().
  The most apparent one is the no visible binding for global variable "variable_name". This is what I wrote in my notes: The “no visible binding” note is a peculiarity of using dplyr and unquoted variable names inside a package, where the use of bare variable names looks suspicious. You can add either of these lines to any file below R/ to eliminate this note (such as the package-level documentation file described in Section 16.7):

# option 1 (then you should also put utils in Imports)
utils::globalVariables(c("english", "temp"))
# option 2
english <- temp <- NULL

• Not a problem, but in the issue description you have a to do point that asks to add a description tag, and this tag is missing.
• Fix the following warning that popped up when running the function: Warning messages: 1: Using one column matrices in filter() was deprecated in dplyr 1.1.0. ℹ Please use one dimensional logical vectors instead. ℹ The deprecated feature was likely used in the fistools package. Please report the issue to the authors. This warning is displayed once every 8 hours. Call lifecycle::last_lifecycle_warnings() to see where this warning was generated. 2: There was 1 warning in stopifnot(). ℹ In argument: min = min(distance, na.rm = TRUE). Caused by warning in min(): ! no non-missing arguments to min; returning Inf 3: There was 1 warning in stopifnot(). ℹ In argument: min = min(distance, na.rm = TRUE). Caused by warning in min(): ! no non-missing arguments to min; returning Inf

[soriadelva]

Some small extra comments (I can't highlight them in the code since I already turned in my review):

• if(sf_id == "sf_id"){ } else { sf_data <- sf_data %>% dplyr::mutate(sf_id = as.character(sf_data[[sf_id]]))}
  --> You can drop the first part and make it a negative if statement: if(sf_id != "sf_id"){ sf_data <- sf_data %>% dplyr::mutate(sf_id = as.character(sf_data[[sf_id]])) }

• Typo in title: Connect seperate line parts into 1 line --> Connect separate line parts into 1 line

• Worth checking: the sf package already contains a function st_line_merge, maybe this already does what you want to accomplish or you can apply it in your code?

[SanderDevisscher]

I didn't know sf::st_line_merge() existed 🤦. I'll check if it does what I need => ifso this PR can be closed

[SanderDevisscher]

Uit mijn beperkte test 🔬 blijkt dat het resultaat van beide functies esthetisch vergelijkbaar is (zie kaartje en code onder). Maar er zijn wel wat verschillen:

• Mijn functie laat de gebruiker toe zowel linestrings als multilinestrings aan te leveren terwijl st_line_merge() enkel multilinestrings accepteert.
• Het resultaat van mijn functie is steeds een linestring terwijl st_line_merge() een multilinestring als geomtery (die je aanleverede) returned. Hierdoor moet je het resultaat nog casten naar linestring en je geometry herberekenen om er sommige handelingen mee te doen.
• st_line_merge() behoud wel alle data terwijl mijn functie enkel de id en geometry exporteert

devtools::load_all()

random_monotonic_linestring <- function(n_points = 3) {
  x <- sort(runif(n_points, min = 4, max = 6))
  y <- sort(runif(n_points, min = 50, max = 52))
  cbind(x, y)
}

# Create two MULTILINESTRING geometries, each consisting of 2 monotonic linestrings
multi1 <- sf::st_multilinestring(list(
  random_monotonic_linestring(3),
  random_monotonic_linestring(4)
))
multi2 <- sf::st_multilinestring(list(
  random_monotonic_linestring(2),
  random_monotonic_linestring(5)
))

sf_data <- sf::st_sfc(multi1, multi2) %>%
  sf::st_sf(id = c("a", "b")) %>%
  dplyr::mutate(label = as.factor(dplyr::row_number()))


pal <- leaflet::colorFactor(palette = "RdBu", levels = sf_data$label)

plot <- leaflet::leaflet() %>%
  leaflet::addTiles() %>%
  leaflet::addPolylines(data = sf_data, color = ~pal(label), weight = 5, opacity = 1)

# connect the line parts
sf_data_connected <- aggregate_lineparts_sf(sf_data, "id")
sf_data_connected2 <- sf::st_line_merge(sf_data, directed = TRUE)

test <- sf_data_connected2 %>% 
  dplyr::filter(. == sf_data$.)

# add sf_data_connected to plot
plot <- plot %>%
  leaflet::addPolylines(data = sf_data_connected, color = "black", weight = 2, opacity = 0.5) %>% 
  leaflet::addPolylines(data = sf_data_connected, color = "red", weight = 2, opacity = 0.5)

plot

=> conclusion I'll fix the requested changes !!

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 82 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (65ba810) to head (a9c04ff).
Report is 106 commits behind head on main.

Files with missing lines	Patch %	Lines
R/aggregate_lineparts_sf.r	0.00%	82 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##            main     #58    +/-   ##
======================================
  Coverage   0.00%   0.00%            
======================================
  Files         16      20     +4     
  Lines        815    1034   +219     
======================================
- Misses       815    1034   +219     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

stale