Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Addis Ababa and Add Partial GTFSFlex Implementation (#156) #159

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

amenk
Copy link

@amenk amenk commented Feb 2, 2020

  • Mostly adding Addis Ababa Scripts
  • Also add transitfeedflex Module to support Minibus System in Addis Ababa

Copy link
Collaborator

@nlehuby nlehuby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @amenk

@@ -0,0 +1,35 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All config should come with some regression tests, so we can make sure our future changes in osm2gtfs do not break your own implementation.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will probably not be able to add those from my current limited python skills. Help is appreciated.

elif 'route_master' in line.tags and line.tags['route_master'] == "share_taxi":
route_type = "Bus"
route_suffix = " (Minibus)"
flex_flag = 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if flex_flag is a flag, can we use a Boolean ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably yes :-)

elif 'route_master' in line.tags and line.tags['route_master'] == "share_taxi":
route_type = "Bus"
route_suffix = " (Minibus)"
flex_flag = 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the idea of it being hard-coded. Can we rather have that information as an attribute on the route / route_master in OSM ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible, need to discuss with the OSM community how it is mapped best on OSM side.


flex_flag = None
if 'route_master' in line.tags and line.tags['route_master'] == "light_rail":
route_type = "Tram"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why you need to override the route_type here 🤔

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In OSM we tagged it as light_rail, and I was thinking it needs to be mapped to Tram in GTFS; but I am not sure

@@ -0,0 +1 @@
Adds Flexible Pick Off / Drop Off (Part of the GTFSFlex specification)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 That's really cool to add this feature!

Can you please add a link to the documentation of the format ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

long time no feedback, sorry :-)
I think it is described here: https://github.com/MobilityData/gtfs-flex/blob/master/spec/reference.md

trip_gtfs.AddFrequency(
"05:00:00", "22:00:00", ROUTE_FREQUENCY * 60)

if 'duration' in a_route.tags:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the ci_abidjan creator also use duration and interval tags for the same purpose 🤔 I wonder if / how we can mutualize the implementations to avoid code duplication.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, how could we do this?

@amenk
Copy link
Author

amenk commented Oct 22, 2022

Long time no see - sorry, I was kind of pushed back by the requirements for regression tests and also we did not work for some while on the project.

We want to bring this pull request forward :-) Unfortunately I have to confess that I only limited Python skills, so might need some support. But we will try our best to get this ready for merge.

@amenk
Copy link
Author

amenk commented Oct 22, 2022

Actually I am nowadays no longer sure if OpenTripplanner v2 (which we use for routing) supports those continuous_pickup + continuous_drop_off fields.

It's also not really documented any more in the spec.

Ref: MobilityData/gtfs-flex#70
opentripplanner/OpenTripPlanner#2603

@amenk amenk changed the title Add Addis Ababa and Add Partial GTFSFlex Impelmentation (#156) Add Addis Ababa and Add Partial GTFSFlex Implementation (#156) Oct 22, 2022
@derhuerst
Copy link

derhuerst commented Oct 24, 2022

Actually I am nowadays no longer sure if OpenTripplanner v2 (which we use for routing) supports those continuous_pickup + continuous_drop_off fields.

For better discoverability, this is Leonard's reply on the OTP mailing list:

continuous_pickup/dropoff is currently not supported by OTP2.
There is however an open PR that would need some polishing to get merged: opentripplanner/OpenTripPlanner#3457
Several people have expressed interest in taking over from the original author but so far no-one has stepped forward.

(Sorry if this is irrelevant, I'm a random lurker on this Issue.)

@hannesj
Copy link

hannesj commented Oct 25, 2022

It's also not really documented any more in the spec.

It is part of the main GTFS specification https://github.com/google/transit/blob/master/gtfs/spec/en/reference.md#stop_timestxt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants