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

[WIP] Issue 30 - Overhauled data structure #84

Closed
wants to merge 3 commits into from
Closed

[WIP] Issue 30 - Overhauled data structure #84

wants to merge 3 commits into from

Conversation

pantierra
Copy link
Contributor

Follow up to PR #70
Asked to be re-requested by @AltNico in https://github.com/AltNico/osm2gtfs/pull/1

@@ -61,7 +61,8 @@ def add_stops_to_schedule(self, schedule, data):
stops = data.get_stops()
stops_by_name = {}

for a_stop in stops.values():
for a_stop_id, a_stop in stops.items():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment in #70 by @nlehuby:

@xamanu your patch on the Accra creator is not ok : it alters the structure of the GTFS stop_id.

Can you help me to fix it, please?

My problem is that on my local machine, Accra is not going through (with the master branch).
With the new structure it threw the error of not getting the osm_id, with the fix it gets it and again the same error is presented to me, as with the old structure. Probably I'm not seeing the whole picture and I would kindly ask to for your support on this particular issue. Thanks a lot!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure !

What is your error message in the master branch ?
Did you refresh the cache to change the data structure ?

With your fix, the stop_id (in the generated stops.txt file) looks like node/5025827612 whereas it was looking like 5025827612 in the master branch.
I did not find yet why, but no stop_times.txt and frequencies.txt are generated in the current branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to check it again. Will do and report.

Regarding the stop name: In osm2gtfs we thought using the format "node/OSM_ID" and "way/OSM_ID" because there can be stops marked as points but also as polygon (Think about the whole bus shelter). But this is only the internal handling of osm2gtfs. Of course the stop_id can be set to anything, what you prefer, but this should happen inside the special city implementation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep, I do agree : "node/OSM_ID" is a better format for stop_id than "OSM_ID" alone.

But Accra GTFS data is already in use, so we cannot change all the stop_id ...
This is already done in the Accra trip creator : https://github.com/xamanu/osm2gtfs/blob/f09f2857d2d1911428203e7a9260a3d10a391044/osm2gtfs/creators/accra/trips_creator_accra.py#L76

@pantierra pantierra changed the title Issue 30 Issue 30 - Overhauled data structure Oct 25, 2017
@prhod
Copy link
Collaborator

prhod commented Nov 19, 2017

@xamanu I tried to run Accra tests on this branch, but I have some troubles ...
I made PR https://github.com/xamanu/osm2gtfs/pull/1 on your branch to continue the fix on stop_id and an issue on Itineray object misuse.
The next two issues I encountered is :

  • all the Itineraries are included in every Line object. Any idea ?
  • the trips.txt file is not written. I'll look at this one ASAP.

@AltNico
Copy link
Collaborator

AltNico commented Nov 20, 2017

@prhod

I tried to run Accra tests on this branch, but I have some troubles ...
I made PR xamanu#1 on your branch to continue the fix on stop_id and an issue on Itineray object misuse.

Thanks for testing the branch and working on it! Actually I'm working on it, too, and as such the changes shown here are a little bit outdated. I did not find the time to upstream everything yet and therefore all changes are in a large messy branch creators-nicaragua, but I will start upstreaming the changes in atomic pull requests next week.

all the Itineraries are included in every Line object. Any idea ?

Yeah, I also noticed this but did not find the reason yet. I'll investigate this today.

the trips.txt file is not written. I'll look at this one ASAP.

For me this actually worked, but I'll try to help you when I finished with the Nicaraguan work.

@AltNico
Copy link
Collaborator

AltNico commented Nov 20, 2017

@prhod

Yeah, I also noticed this but did not find the reason yet. I'll investigate this today.

OK, learned another thing about Python:

The value you are defining is not an instance field for your class, its more like a static field. But python doesn't mind if you access this field from instances. So, even if you access this field from instances, it is not a different list for each instance. Basically, you are appending to the same list every time the method is called.

Source: https://stackoverflow.com/a/10085764

Sorry for the inconveniences.

@AltNico AltNico changed the title Issue 30 - Overhauled data structure [WIP] Issue 30 - Overhauled data structure Nov 22, 2017
@pantierra
Copy link
Contributor Author

Closing as there has been a lot of things happening in https://github.com/mapanica/osm2gtfs

@pantierra pantierra closed this Nov 28, 2017
@pantierra pantierra deleted the issue-30 branch December 23, 2017 23:05
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