Skip to content

Conversation

@zandemax
Copy link
Contributor

As mentioned in #115, it would be great to process the transfers.txt from the GTFS spec.

For this, I have added:

  • a new Struct RawTransfer and StopTransfer
  • a new field transfers to RawGtfs
  • a new field transfers to Stop in Gtfs
  • modified to_stop_map to process transfers and add them to the corresponding stop

I would appreciate your comments and feedback on this PR, and thanks already for your time and work on this library!

Copy link
Collaborator

@Tristramg Tristramg left a comment

Choose a reason for hiding this comment

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

Hello, thank you so much for this contribution, and its very high quality.

I’m suggesting one change to check the reference to destination stop.

Also, we’ll need to bump the version in the Cargo.toml

--

More complicated: if we have a station, the transfer applies to the child stops. I don’t think we need to generate all the extra transfers (this would very specific on how you want to manipulate it in your pathfinding). I’d say nothing to do here, but I wanted to be sure that you have considered this situation.

--

And just for curiosity, in which context do you use the library? Are there some other need (maybe not directly related to gtfs, but concerning transit data and its use?)

@zandemax
Copy link
Contributor Author

zandemax commented Feb 18, 2022

Thanks for having a look and replying so quickly!

I didn't increase the version in cargo.toml yet, since i was not sure what your versioning/release process was like, but I incremented it now :)

Regarding the station/child problem, I think I would leave that up to the user of the library to implement, since as you said this depends on what you want to do in the end, but it is a good thing to consider!

I am currently working on a small project for which I am implementing a slightly modified version of RAPTOR, where I am using this library mostly for parsing GTFS and then turning it into my own representation for routing on. If you are curious I'd be happy to send you a link to the repo once I release it, currently its still in a bit of an early stage, e.g. missing transfers :)

@Tristramg
Copy link
Collaborator

Thank you for all the possible leads, and sorry that I hadn’t have a deeper look into what my suggestion implied 😅

I was a bit busy. I will dig into it to have a more informed opinion

@zandemax
Copy link
Contributor Author

No worries! I also wasn't quite aware of it until I tried to implement it :)

@Tristramg
Copy link
Collaborator

Hello,

I discussed with @antoine-de and you are completely right about the mutability hell introduced by Arc.
However, we are not fond of introducing unsafe code just for this, and the third version is too cumbersome.

So let’s merge the way you did it, as it will actually be helpful.

We will explore a bit radical solution using things like https://docs.rs/generational-arena/0.2.8/generational_arena/
Let’s see where that brings us, maybe nowhere, maybe it will solve the problems with the translations #74 . And in any case its fun to try out approaches used for video games.

@zandemax
Copy link
Contributor Author

Sounds good to me, I'm also not a big fan of introducing unsafe or RefCells here.

The library you linked sounds also very interesting, thanks for pointing it out!

for transfer in raw_transfers {
stop_map
.get(&transfer.to_stop_id)
.ok_or(Error::ReferenceError(transfer.to_stop_id.to_string()))?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

❤️

Co-authored-by: Tristram Gräbener <[email protected]>
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.

2 participants