Use Index<Stop> instead of Arc<Stop>: Id as integer #123
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
after having given thought about this, I think the |
antoine-de
force-pushed
the
generational_index
branch
2 times, most recently
from
2fbd059 to
627544e
Compare
POC to see if the ergonomics are ok or not
antoine-de
force-pushed
the
generational_index
branch
from
627544e to
dc30a34
Compare
antoine-de
added a commit
that referenced
this pull request
Mar 23, 2022
Same as #123 but without generational_indexes. I think the indexes are really great for performance since: * all structures are then `Vector` (great random access and cache friendly iterations) * the ids are short (integer vs string) I don't think we need those performance in this crate though. Having only typed index (a think wrapper over the real gtfs identifier) would be more convenient I think. It would: * ease debug (easy to print the gtfs identifier, instead of having a meaningless integer) * ease serialisation (same, we can serialize the string right away) * be a more more close to the gtfs representation This is still *very* early WIP, I'm not convinced at all by the ergonomics, I'd like to keep the property of the Index in #123 that the `Id` have at least existed at one point (even if I don't plan to ensure this if an object is deleted).
3 tasks
antoine-de
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
POC to see if the ergonomics are ok or not
linked a bit to #116 (comment)
Remove
Arc<Stop>in favor ofIndex<Stop>inStopTimesThis has the downside of needing the whole
gtfsstructure to access the stop's fields, but now we can have a mutable model easily.I'd like to try migrating a lib using gtfs_structures (maybe gtfs-to-geojson) to check this, but I open this draft to see if anyone has thoughts/ideas about this.
One down side of this is that I used https://gitlab.com/tekne/typed-generational-arena that is not very active (but it's so nice to have typed index 😻 )