Skip to content

Conversation

ivan-eguidazu
Copy link

agency_id on routes.txt is only required if multiple agencies are defined in agency.txt
Instead of leaving agency_id as None on routes.txt, we set it to the first agency_id in case there is only one agency (univocal)

one specific test has been added in order to test this new feature

…pletes this value instead of leaving it as None

agency_id on routes.txt is only required if multiple agencies are defined in agency.txt
Instead of leaving agency_id as None on routes.txt, we set it to the first agency_id in case there is only one agency (univocal)
…ew feature of parent commit

It loads a GTFS where agency_id is not defined on routest.txt, and check whether agency_id has been completed as expected
Copy link
Owner

@jarondl jarondl left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!

# Empty row.
continue

if gtfs_class == Agency:
Copy link
Owner

Choose a reason for hiding this comment

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

I wonder if this would be cleaner in a post-processing fix up. After reading all of the tables, you can check the number of agencies, and issue an update.

I haven't touched this code or sqlalchemy for a very long time, but it should be doable. Perhaps something like

# find if there is only one agency.. Based on:
select(Agency.agency_id).all()
# update it if you need to
if len(agency_ids == 1):
  session.update(Route)
    .where(routes.c.agency_id.is_(None)
    .values(agency_id=agency_id)

What do you say? Does that make sense?

continue

if gtfs_class == Agency:
# agency_id on routes.txt is only required if multiple agencies are defined in agency.txt
Copy link
Owner

Choose a reason for hiding this comment

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

If we do follow this route, can you add to the comment that agencies are always processed before routes? (That's true, but a casual reader might miss this)

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