dodgr_dists with numeric node indices gives unexpected results

[luukvdmeer]

Hi Mark! I was running dodgr_dists on a simple graph in which the from and to nodes of the edges are encoded by numeric values, rather than by characters as is used in most of your examples. This gives some strange results which I am not sure if they are intended, see the reprex below:

library(dodgr)

graph <- data.frame (
  from = c (1, 2, 2, 2, 3, 3, 4, 4),
  to = c (2, 1, 3, 4, 2, 4, 3, 1),
  d = c (1, 2, 1, 3, 2, 1, 2, 1)
)

# This does not work.
dodgr_dists(graph, from = 1, to = 2)
#> Error in dimnames(x) <- dn: length of 'dimnames' [2] not equal to array extent

# This works.
dodgr_dists(graph, from = 1L, to = 2L)
#>   2
#> 1 1

# However, I don't think it works as expected.
# For example if we let the second edge start from node 3 instead of node 2.
# Then computing distance between 1 and 2 actually computes between node 1 and node 3.
graph <- data.frame (
  from = c (1, 3, 2, 2, 3, 3, 4, 4),
  to = c (2, 1, 3, 4, 2, 4, 3, 1),
  d = c (1, 2, 1, 3, 2, 1, 2, 1)
)

dodgr_dists(graph, from = 1L, to = 2L)
#>   3
#> 1 2

# For the record: Everything works well when nodes are character encoded.
graph <- data.frame (
  from = as.character (c (1, 3, 2, 2, 3, 3, 4, 4)),
  to = as.character (c (2, 1, 3, 4, 2, 4, 3, 1)),
  d = c (1, 2, 1, 3, 2, 1, 2, 1)
)

dodgr_dists(graph, from = "1", to = "2")
#>   2
#> 1 1

^{Created on 2024-09-26 with reprex v2.1.1}

[mpadge]

Thanks for noting that @luukvdmeer. As background and general advice: One design aim of this package was to be "smart" about from and to arguments, and "cleverly" match them to desired values in any many arbitrary ways as possible. They can be integers, numeric values, character vectors, or matrices of coordinates. I thought that was clever. I would never do that again, and would strongly recommend nobody else do anything like that. The world is much easier when you pretend that R is a rigidly-typed language. That said ...

The error you observed was this simple one-line fix. More importantly, the commits above update the docs to clearly state the intention of from and to values when entered as integer-ish vectors:

dodgr/R/dists.R

Lines 49 to 59 in 648129f

	#' The `from` and `to` parameters passed to this function can be either:
	#' \itemize{
	#' \item Single character vectors precisely matching node numbers or names
	#' given in `graph$from` or `graph$to`.
	#' \item Single vectors of integer-ish values, in which case these will be
	#' presumed to specify indices into \link{dodgr_vertices}, and NOT to
	#' correspond to values in the "from" or "to" columns of the graph.
	#' \item matrices or equivalent of longitude and latitude coordinates, in which
	#' case these will be matched on to the nearest coordinates of "from" and "to"
	#' points in the graph.
	#' }

I also added some tests to demonstrate why you see the results you gave above. The vertex map is made by sequentially crawling the from values given in graph construction. In your first example, these are indeed sequential: 1:4, but in your second, they are 1, 3, 2, 4, so a value of to = 2 will map on to the second of these, and return 3. Hopefully that's now all explained much better in the docs, but let me know if you can see any room for improvement.

Thanks as always for helping to make the package better!

[luukvdmeer]

Yes, this docs now make it clearer! I see it is not easy indeed to try to anticipate all different ways people may specify the from and to nodes ;)

[agila5]

I thought that was clever. I would never do that again, and would strongly recommend nobody else do anything like that. The world is much easier when you pretend that R is a rigidly-typed language.

I will add this quote to my notes for a class on the R software ahahhaha