Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

67 try ingesting branching routes as multilinestring objects #72

Conversation

mbjackson-capp
Copy link
Contributor

Ingesting branching routes as MultiLineString objects was successful. Ingestion script for TransitStop and TransitRoute is in good enough condition to merge into main.

Matthew Jackson added 5 commits May 6, 2024 17:18
When run from the command line, the file extract_scheduled_gtfs.py
should now iterate through the URLs for the static GTFS feeds for
our three cities, transform the data to prepare it for storage,
and write it to the PostGIS backend.

Two small issues to consider before merging:

1) Many stop and route names go past the 30-character limit.
When this happens, the entire upload for that stop or route is
skipped.
We may want to either consider extending that limit or truncating
names for long stops (e.g. by taking the first 27 characters and
adding "..." if the name is too long).

2) Some large bus systems seem to have multiple rows with the same
stop_id, perhaps for different directions of travel or routes that
pick up at the same stop. We should investigate further to see
if this is fine or would cause issues linking to other data.
Delete unneeded single-LineString version of procedure to handroll
geometrized routes (since we committed to MultiLineString approach).
Clean up other comments and print messages here or there.
Copy link
Contributor

@jimenasalinas jimenasalinas left a comment

Choose a reason for hiding this comment

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

The code looks good to me! Thank you for resolving yesterday's issue

@jimenasalinas
Copy link
Contributor

For future iterations (not urgent) we may want to separate ingestion from extraction to allow for tests to work

@meganhmoore
Copy link
Contributor

meganhmoore commented May 7, 2024

Can we get a screenshot of tests passing if its too much to migrate tests to the route_rangers_api/tests.py?

)
print(
f'Observation created: {row["city"]}, {row["stop_id"]}, '
f'{row["stop_name"]}, Point({row["stop_lat"]},{row["stop_lon"]})'
f", mode {obs.mode}"
)
print(obs)
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

Good for this stage, but let's keep in mind for later to get more specific try-excepts

return BUS


def handroll_multiline_routes(city: str, feed) -> gpd.GeoDataFrame:
Copy link
Contributor

Choose a reason for hiding this comment

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

I get this is fine for now, but I don't find the name of the function particularly clear on what should do.

Copy link
Contributor

@JPMartinezClaeys JPMartinezClaeys left a comment

Choose a reason for hiding this comment

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

This looks good to me

@meganhmoore
Copy link
Contributor

Looks good to me! I made some updates with my latest PR that migrated your test file to the app/route_rangers_api/tests.py file so that when the imports tried to call these functions that rely on django models they wouldn't immediately error.

That being said, most of those tests are currently being skipped and outdated imports were commented out (https://github.com/uchicago-capp-30320/RouteRangers/blob/main/app/route_rangers_api/tests.py#L11-L16) so if you want to make an issue to go back and update those tests, delete the ones that are no longer needed, etc. please do! doesn't have to be high priority though

@mbjackson-capp mbjackson-capp merged commit 66df4ad into main May 8, 2024
1 check failed
@mbjackson-capp mbjackson-capp deleted the 67-try-ingesting-branching-routes-as-multilinestring-objects branch May 8, 2024 15:28
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.

Try ingesting branching routes as MultiLineString objects
4 participants