feat: 1312 add missing data to the pmtiles#1328
Conversation
qcdyx
changed the title
| if not default_agency_name: | ||
| default_agency_name = agency_name | ||
|
|
||
| if not agencies and default_agency_name: |
There was a problem hiding this comment.
This condition is never true, as if the default_agency_name is set differently than "", the previous loop will have added at least one entry in the agencies dictionary.
| agencies = {} | ||
| default_agency_name = "" | ||
| for row in self._read_csv(self.get_path(AGENCY_FILE)): | ||
| agency_id = row.get("agency_id") or "default" |
There was a problem hiding this comment.
[question]: Is it possible not to use the key "default"? It might be unlikely that an agency set the ID to "default," but it's possible.
| agency_id = route.get("agency_id") or "default" | ||
| agency_name = agencies.get(agency_id, agency_id) |
There was a problem hiding this comment.
I think it is preferable to use an empty string rather than "default".
There was a problem hiding this comment.
Agreed. Using "default" as a key could conflict with a valid agency_id. It's better to use an empty string for the default agency when agency_id is not specified.
There was a problem hiding this comment.
I find that default_agency_name is not useful in _load_agencies anymore, removed.
…ilityData/mobility-feed-api into 1312-add-missing-data-to-the-pmtiles
davidgamez
left a comment
davidgamez
left a comment
There was a problem hiding this comment.
Great work, I just added a non-blocking suggestion.
3 participants
Summary:
Closes #1312
Expected behavior:
Testing tips:
Provide tips, procedures and sample files on how to test the feature.
Testers are invited to follow the tips AND to try anything they deem relevant outside the bounds of the testing tips.
Please make sure these boxes are checked before submitting your pull request - thanks!
./scripts/api-tests.shto make sure you didn't break anything