saferactive / trafficalmr

R package to support road safety and traffic calming measures
https://saferactive.github.io/trafficalmr/
GNU General Public License v3.0
5 stars 1 forks source link

segment argument in osm_consolidate does not work as expected #47

Closed Robinlovelace closed 4 years ago

Robinlovelace commented 4 years ago

The docs say:

#' @param segment numeric, max length of segments in metres

This implies that longer segments will results in fewer output objects. However, the opposite is true, as shown below on the osm_cleaning branch:

> nrow(x) / nrow(osm_consolidated)
[1] 1.258448
> osm_consolidated_1000m = osm_consolidate(x, segment = 1000)
  |++++++++++++++++++++++++++++++++++++++++++++++++++| 100% elapsed=00s  
Warning message:
In st_cast.sf(xmlsB, "LINESTRING") :
  repeating attributes for all sub-geometries for which they may not be constant
> nrow(x) / nrow(osm_consolidated_1000m)
[1] 1.408046
> osm_consolidated_200m = osm_consolidate(x, segment = 200)
  |++++++++++++++++++++++++++++++++++++++++++++++++++| 100% elapsed=02s  
Warning message:
In st_cast.sf(xmlsB, "LINESTRING") :
  repeating attributes for all sub-geometries for which they may not be constant
> nrow(x) / nrow(osm_consolidated_200m)
[1] 0.8423815

Was this intended @mem48 ? seems like a bug to me.

mem48 commented 4 years ago

I'm not sure what your point is? Smaller segments will result in more rows as roads get broken up into pieces.

Robinlovelace commented 4 years ago

I'm not sure what your point is? Smaller segments will result in more rows as roads get broken up into pieces.

True that, was too late on a Friday afternoon... Was aiming to calculate the % of the number of features in the new object but was doing old/new rather than new/old - my bad, good to have examples showing that that's how it work and to 'kick the tires'. Great function btw.