DRAFT: Full translation implementation#155
DRAFT: Full translation implementation#155kylerchin wants to merge 15 commits intorust-transit:mainfrom
Conversation
|
Documentation will be added once everything is set! Thank you to Andrew especially for spending hours with me to whiteboard out data structures and coming up with an implementation. Attached is our whiteboard understanding and possible data structure examples of implementation:
|
|
Should we keep the HashMap like this? Or should we use the trait system? or both? What direction should the lookup mechanism take? I personally would be alright with the current structure and a trait lookup mechanism, providing options would make the library more friendly for developers. |
|
I was thinking for the Refer to the record_sub_id’s requirement in description, though they point out agencies can add additional fields that might invalidate the specified requirements. |
antoine-de
left a comment
antoine-de
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
Sorry for the time taken to review this, I don't have much time for the moment, and this is quite a big chunk to review 😁
by curiosity, why didn't you use the associated types used in @Tristramg POC? There are not convenient to use?
I'll try to properly review this a bit later, this is my notes after the first read.
| let mut possible_translations:HashSet<(TranslatableField, LanguageTag)> = HashSet::new(); | ||
|
|
||
| for row in raw_translations { | ||
| if let Ok(language_tag) = LanguageTag::parse(row.language.as_str()) { |
There was a problem hiding this comment.
I know it's a POC, but I'll just add a comment to not forget that this should either stop on error or log something
There was a problem hiding this comment.
This isn't a big issue to implement for us, because every agency I've seen so far does use valid language strings
| raw_translations: Vec<RawTranslation>, | ||
| ) -> ( | ||
| HashMap<TranslationLookup, String>, | ||
| Vec<(TranslatableField, LanguageTag)> |
There was a problem hiding this comment.
I think I might be missing something, but I'm not really sure I understand why this TranslatableField is used?
Can't we just have the HashMap<TranslationLookup, String>
Maybe it's because there isn't the translation mechanism yet (which is fine, but it makes it more difficult to understand this code)
There was a problem hiding this comment.
This creates the summary together because it's cleaner to do it in the same iter instead of writing a second iter in the primary gtfs. I can add some comments to explain what's going on if you'd like.
|
We plan on implementing traits properly in a bit |
|
|
||
| //oh no... i need trip_id... but that's contained in the parent... | ||
|
|
||
| // should I add trip_id to stop_times? |
There was a problem hiding this comment.
I need assistance here, can I add Arc to stop_times to allow headsign translation to work?
Los Angeles Metro and other agencies use dynamic headsigns that change along a route, thus translation of stop_times is important here.
CC: @antoine-de @Tristramg @CallMeJai @AndrewHC36
3 participants
This is a collaborative effort between the Rust-Transit Maintainers and the Catenary Transit Initiatives https://catenarymaps.org team, a nonprofit designing open-source public transport software. We seek comments from the community and maintainers.
Team members responsible:
@CallMeJai @AndrewHC36 @kylerchin
Thank you to @Robbe7730 for the original idea of RawTranslations
THIS IS A WORK IN PROGRESS DRAFT AND WILL BE UNMARKED ONCE READY
This discussion originated at https://github.com/orgs/catenarytransit/discussions/2
Key points of discussion:
Final notes:
Note that as GTFS is an evolving specification, the contents of the translatable field will change over time.