bouzaghrane commented 3 years ago

What

This PR is to add capability to create time-aware urbanaccess transit and walk networks.

bouzaghrane commented 3 years ago

It's unclear to me why checks on my commits keep on failing. Is it a PEP8 issue? It seems like all other tests pass. @sablanchard @smmaurer

smmaurer commented 3 years ago

@bouzaghrane Yup, looks like just PEP8/pycodestyle. Here are the lines that get flagged: https://travis-ci.org/github/UDST/urbanaccess/jobs/761002604#L1258-L1284

You can do pip install pycodestyle and run it locally too, which makes it easier to confirm that everything clears. From the root directory of the repo just run pycodestyle urbanaccess

bouzaghrane commented 3 years ago

Looks like the checks passed for all but python 3.5. Wondering why..

smmaurer commented 3 years ago

It looks like an installation problem that's unrelated to these changes -- Conda doesn't find Python 3.5-compatible versions of the dependencies, so the tests can't continue.

I'll work on diagnosing this, and we can either patch the Travis script or switch over to GitHub Actions like we've done for Pandana, ChoiceModels, and UrbanSim Templates..

smmaurer commented 3 years ago

Hi @sablanchard and @bouzaghrane!

Sam, Amine mentioned that this is ready for review. Thanks Amine!!

The changes in this PR are to the create_transit_net() function: (1) adding a time_aware parameter that if True adds arrival and departure time columns to the output network, and (2) adding a timerange_pad parameter to optionally extend the timerange by a particular amount.

There are a couple of reasons for the first addition. Including schedule times allows us to create a single large integrated network that we can later break into time-of-day segments on the fly as needed. It also paves the way to potentially include trip schedules in the routing itself.

The reasoning behind timerange_pad is related. If we have an 8am-10am network, we're not guaranteed to be able to route an agent who leaves at 9:30. It will only work if their endpoint can be reached before 10am. Padding the time range would guarantee that we could find routes for every agent who departed within the notional time window. For example, the 8am-10am network could actually be 8am-12pm, and every agent leaving between 8 and 10 would complete their trips. It's helpful to think of the padding separately from the original time range because the optimal padding will vary from region to region.

Feedback from me:

[ ] The new function arguments should probably go at the end of the existing argument list. This ensures we won't break existing code that calls the function with positional (rather than named) arguments.
[ ] Let's add a unit test confirming that the time_aware parameter works as designed. Looks like it could go here: test_gtfs_network.py
[ ] And i'll open a separate PR to fix the test glitch discussed above

@sablanchard, any additional feedback from your side?

sablanchard commented 3 years ago

Working with @bouzaghrane offline we decided to implement the functionality in this PR in a different way in this new PR: https://github.com/UDST/urbanaccess/pull/87 thus this PR is superseded by https://github.com/UDST/urbanaccess/pull/87 and will be closed.

UDST / urbanaccess

Make the transit networks time aware #83

What