Fixed wrong assumption in implied_oneWay() function

[cayetanobv]

Hi,

I fixed a wrong assumption(*) in implied_oneWay() function (see below OSM info about it):

if (key == "highway"
            && (value == "primary"
                || value == "secondary"
                || value == "tertiary")) {
        m_oneWay = "NO";
        return;
    }

(*) Following OSM wiki, oneway in these key/value combinations can be equal to yes or no:

• http://wiki.openstreetmap.org/wiki/Tag:highway%3Dprimary
• http://wiki.openstreetmap.org/wiki/Tag:highway%3Dsecondary
• http://wiki.openstreetmap.org/wiki/Tag:highway%3Dtertiary

This assumption makes that a lot of roads with oneway=yes changes automatically to oneway=no (and this is a problem in PgRouting). You can see this example:

Original info (OSM):

New info processed with osm2pgrouting:

Thanks,

Cayetano

[cvvergara]

Then should it be like this?

  if (key == "highway"
            && (value == "primary"
                || value == "secondary"
                || value == "tertiary")) {
        m_oneWay = "YES";
        return;
    }

[cayetanobv]

Hi @cvvergara ,

In this PR I removed this IF statement because any of these key/value combinations (highway=primary, highway=secondary, highway=tertiary) can be either oneway=yes or oneway=no. This IF statement is a wrong assumption in any case.

Thanks,

[cvvergara]

But what happens when oneway tag is not there?

[cayetanobv]

When oneway tag does not exist osm2pgrouting assign 0 (this is equal to unknown). I think this is correctly solved in osm2pgrouting: https://github.com/pgRouting/osm2pgrouting/blob/master/src/osm_elements/Way.cpp#L311

In PgRouting oneway=0 (oneway=unknown) is the same as oneway=2 (oneway=NO).

[cvvergara]

Adding more links: http://wiki.openstreetmap.org/wiki/Talk:Key:oneway#Default.3F http://wiki.openstreetmap.org/wiki/OSM_tags_for_routing#Oneway

@dkastl Can you comment on this? It looks to me that he is right. (but my experience using real data is very limited)

[cayetanobv]

Hi @cvvergara @dkastl , do you have any new ideas about this PR?

[dkastl]

Honestly I haven't looked into detail about tagging streets oneway. Even if there are Wiki pages about this, they look a bit confusing, and I guess that there are cases where mappers might do something wrong.

I would vote for the PR of @cayetanobv , because - as he says - we currently do some assumption, that may not be correct. Let's remove the assumption, make it more simple, and see, if there will be complaints, that oneway was handled wrong.

[cvvergara]

@cayetanobv at what time is a good time for you so we can work together he merges? I am on this gitter: https://gitter.im/pgRouting/pgrouting with 3 PR I need to work it out with you

[cvvergara]

@cayetanobv I made the three PR, but to be v2.3.3 Before I make the new release, can you verify that works as expected?

[cayetanobv]

Ok @cvvergara , I'm going to test now. By the way, I'm on gitter if you want something (my timezone is UTC+1, Spain).

[cayetanobv]

@cvvergara I tested master branch with new PRs with real datasets and all is Ok. You can release new version 2.3.3.

[cvvergara]

@cayetanobv I wonder if you can try it using a schema

[cayetanobv]

@cvvergara Yes, I used a database schema.