-
Notifications
You must be signed in to change notification settings - Fork 34
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
[Draft] Changes needed to add TTC #577
base: master
Are you sure you want to change the base?
Conversation
except Exception as exc: | ||
print(exc) | ||
print('Ensure tryn-api is running and that you set the TRYNAPI_URL environment variable') | ||
exit(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Library functions like this should raise exceptions when errors occur, so that the caller can handle them as needed. Calls to exit() should be in the top-level scripts like compute_new.py . If the top-level script only should exit in certain cases, you could raise a particular class of exception here and catch that type in the top-level script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, normally it shouldn't be necessary to set TRYNAPI_URL, since docker-compose.yml sets TRYNAPI_URL to http://tryn-api.opentransit.city . This also applies when running scripts from the command line via docker-shell.sh or docker-shell.bat.
continue | ||
if stop_geometry['offset'] > 300: | ||
# Throw as skipping it will result in speed metrics API calls failing | ||
raise Exception( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should raise an exception -- if the offset is ever greater than the threshold, it's not clear what to do to fix it, other than by continuing to increase the threshold until the exception stops being raised. At that point, what is the purpose of having this check at all?
I think it would be better to keep the original behavior and then fix the exception that is being thrown by GraphQL API when stop geometry is not available.
…n metrics API errors
This reverts commit 416846dd3404c8d15cdbd8212ba3d8e357b2ee2a.
… trynapi"" This reverts commit 2500acbf09a78ae2db277f2634f946620521140a.
This reverts commit 235f564.
- add custom default directions - use our fork of remix/partridge to use a patch for buggy CSVs - use the TTC route number for sorting on the route-table
Makes progress on adding TTC. Improves error messages for adding future agencies.
Proposed changes
Replacing the default TRYNAPI_URL is required for running compute_new.py locally; however the error thrown when it is not doesn't suggest this. This PR catches the trynapi request error in trynapi.py and prints an error message that says it could not connect to TrynAPI and asks whether the environment variable was provided and exits. The stacktrace resulting from raising there is very long and not very useful, so it seems reasonable to simply exit.
Tolerance for broken stop geometry is increased from 100 to 300 metres, as to account for route terminals that are moved without the geometry being updated. In the image below the stop was moved farther back as the airport decided that Uber/Lyft needed more space near the arrivals hall.
Skipping over the route's geometry resulted in the dashboard being all grey as the initial metrics API call failed due to an exception thrown in the average speed calculation as the geometry for a stop was missing. Now save_routes.py raises an exception if the stop geometry is bad, which has not caused issues for Muni, Trimet, or TTC.
Continue working on the config for TTC, this PR adds one route and changes the start time for the overnight bus routes.
Implement Add direction groups for routes in agency routeConfig #574, which adds custom default directions, allowing east-west routes to be specified in less lines than custom_directions would require.
...
Link to demo, if any
https://eddy-opentransit.herokuapp.com
(will be pre-computed when #423 is resolved)
...