A sensible default for the directed argument

[luukvdmeer]

Following a discussion that emerged in #46, we should decide on a sensible default for the directed argument when creating networks.

The sfnetwork construction function takes a data frame (sf, data.frame, tbl_df, ..) that specifies the location and possible attributes of the nodes, and a data frame (sf, data.frame, tbl_df, ..) that specifies the edges, at least with a to and from column referring to node indices, and optionally also with a linestring geometry. By default, construction assumes that directed = TRUE. This is the same assumption that tidgyraph and igraph make. I think for this case that 1) preserving the default of the "parent packages" prevents confusion and 2) having a directed = TRUE is most intuitive here, because you are already forced to provide your edges explicitly with a from and to column.

However, sfnetworks also offers the possibility to create a network directly from an sf object with linestring geometries, by using as_sfnetwork.sf. Here, the from and to columns are internally derived from the linestrings that you provide. The first point of the linestring is the from node, and the last point of the linestring is the to node. Here I completely agree with @agila5 and @mpadge that it is more intuitive to have a default of directed = FALSE, especially because, as @mpadge pointed out, simple feature geometries are not intended to have a direction (despite that, there is actually a possibility in sf to reverse the direction of linestring, by using st_reverse, so as a user you do have some influence on the direction of your input).

In summary, I see 3 options, which all have their pro's and con's:

1. Having the directed = TRUE default in all construction functions. This preserves the defaults of igraph and tidygraph, and will probably be most intuitive for users constructing their network by providing an already established network structure in the form of a list of data frames for nodes and edges (with the latter containing to and from columns).
2. Having the directed = FALSE default in all construction functions. This is probably most intuitive for users that let sfnetworks derive the network structure from a single sf object with linestring geometries.
3. Having directed = TRUE for the general sfnetwork() construction function, but using a default of directed = FALSE for the as_sfnetwork.sf() method. This uses the most intuitive default for both cases, but on the other hand might cause more confusion because the default is not the same throughout the whole package.

Comments relating to this discussion in #46:
#46 (comment)
#46 (comment)
#46 (comment)
#46 (comment)
#46 (comment)

[Robinlovelace]

My 10 cents: thinking about users, the one that is most useful depends on what they will need/expect. We haven't seen many use cases outside transport but can think of both directed (river network) and undirected (electricity grid) examples. As long as the directed argument is available and well documented people can easily change.

My thinking, which may be biased by personal preference, would be that 2 (directed = FALSE) is a simple and safe default. I like the idea of 3 but in practice having directed in one and undirected in another function could have unintended negative consequences and increase code complexity and ambiguity. Being explicit is good.

I think 2 with prominent examples of object creation with directed = TRUE is good enough for now and safe enough to try.

[mpadge]

As a 5 cent addendum to your 10 cents @Robinlovelace, and in complete contradiction of that feller who previously insisted:

a default of directed = TRUE requires an awful lot of work, and a completely different approach to routing. Defaults should almost always be directed = FALSE.

an additional option to your list @luukvdmeer would be,

4. Have universal directed = TRUE as your default for all object creation methods, and ensure that construction from sf appropriately duplicates all edges unless otherwise indicated.

This is what dodgr does, by extracting data from Open Street Map, and assuming all edges are to be duplicated unless otherwise flagged as oneway. The explicit flagging is the key, and it is one of the few bits that is inviolably hard-coded: all edges are duplicated as directed edges unless a oneway column indicates otherwise - and that column must be called "oneway".

What this means in practice is that all representations should explicitly flag the directedness of each edge, and in the osmdata case all representations must do this. This general workflow would be very easy to translate through to sfnetworks by insisting that sf representations (via sfnetwork or as_sfnetwork.sf methods) have a "oneway" column, which would default to FALSE throughout.

library (osmdata)
#> Data (c) OpenStreetMap contributors, ODbL 1.0. https://www.openstreetmap.org/copyright
x <- opq ("roxel germany") %>%
    add_osm_feature (key = "highway") %>%
    osmdata_sf () %>%
    osm_poly2line ()
table (x$osm_lines$oneway)
#> 
#>  no yes 
#>  21 143
length (which (x$osm_lines$oneway == "yes")) / nrow (x$osm_lines)
#> [1] 0.05506353

^{Created on 2020-06-08 by the reprex package (v0.3.0)}

And 5% of ways in Roxel are actually one way. This just indicates that this information is essential to any accurate representation of way networks, and that:

1. You (@luukvdmeer) should simply update your roxel data set to include the additional "oneway" column; and
2. Use this column to determine whether or not to duplicate edges, defaulting to duplication unless otherwise indicated.

This is in fact what both igraph and dodgr do under the proverbial hood - all directed edges are in fact represented twice, once for A -> B, and once for B -> A. The simplest interface is to have an explicit flag in your fundamental representation that controls whether or not such duplication occurs. Doing that actually then obviates any need to have an explicit directed option, at least for the sf-methods. That parameter is dumped in favour of an explicit flag within the data themselves.

[Robinlovelace]

Good idea to add the oneway column to the example dataset. Worth opening a new issue on that?

[mpadge]

It's non-trivial, because the front-end code calls the "edges" "from" and "to", but these are sf objects, so ought to be considered as not encoding any "from" and "to" information. A solution would also require renaming these "from" and "to" columns to something else that does not imply directionality. I'd suggest that should first be addressed - and maybe indeed via a separate issue - before modifying the dataset. But and so the first important question is: What should the columns denoting endpoints of the sf objects be called?

• "from" and "to"
• "start" and "end"
• "end1" and "end2"
• "point1" and "point2"
• ?

[luukvdmeer]

The oneway column approach seems very useful indeed! I would see it as an extra option, rather than as a hard requirement. For example, the as_sfnetwork.sf function will have a oneway argument, which you can provide either a vector of booleans of length nrow(sf_obj), or the name of an existing column in sf_obj. When given, the edge duplication will take place as you describe. If sf_obj object has a column named exactly oneway, this will be set automatically as oneway argument. This is similar to how igraph works in shortest path calculations. When the weights argument is NULL, no weights will be used, unless the edges have an attribute (in our case, a column) named weight. Would that make sense?

Regarding the naming of from and to: since sfnetwork sub-classes tbl_graph, those columns will always be called from and to, by design. When constructing a tbl_graph from a nodes and edges data frame, you generally provide the edges data frame containing two columns named from and to. That is why the sfnetworks code also calls them like that. It is possible to provide those columns with other names, as long as they are the first two columns in the edges data frame. However, tidygraph will in that case simply rename them to from and to.

[Robinlovelace]

Belated answer to @luukvdmeer's question

When the weights argument is NULL, no weights will be used, unless the edges have an attribute (in our case, a column) named weight. Would that make sense?

I think the st_length() of the edges is a safe default if there is not weight column. First impression of edge duplication is against it as it increases complexity and may slow things down.