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

Maintenance/gtfs network refactor w unit tests #86

Merged
merged 28 commits into from
Apr 29, 2021

Conversation

sablanchard
Copy link
Contributor

@sablanchard sablanchard commented Apr 23, 2021

Maintenance update with no substantial API changes comprised of code simplification, docstring and print updates, and unit test coverage expansion focusing on gtfs.network.py which include:

  • updates to gtfs.network.py:

    • refactor save_processed_gtfs_data() and load_processed_gtfs_data() by simplifying code, adding more informative prints, and added calendar and calendar_dates as optional tables to supplement changes made in calendar file handling #70
    • refactor edge_impedance_by_route_type() by simplifying code, adding more informative prints, updates to accept new GTFS modes 11 and 12, new validation on input parameters, updates to docstring
    • refactor _interpolate_stop_times() section that uses _check_if_index_name_in_cols() for added clarity and fix edge case where index was removed before merge
    • throw an error when interpolater sees duplicate stop_sequence and unique_trip_id records to aid in debugging
    • modify informative prints on selected trips in _trip_schedule_selector() to be conditional on what input data is utilized
    • refactor create_transit_net() with:
      • minor formatting updates
      • docstring updates
      • print updates
      • simplification of repeated code
      • add prints to clarify when overwrite_existing_stop_times_int or use_existing_stop_times_int are used and dont allow overwrite_existing_stop_times_int and use_existing_stop_times_int to both be True for clarity
      • remove ValueError that could never be triggered
  • general updates:

    • updates contributing guidelines
    • move create_transit_net() and headways() time range value check to its own shared function: _check_time_range_format() with unit tests
    • replace instances of '{}/{}'.format() -> os.path.join()
    • added and modified prints for functions that save and load HDF5 files in particular to simplify hdf5_to_df() key prints
    • address YAMLLoadWarning by replacing yaml.load(f) -> yaml.safe_load(f)
    • add new and updated unit tests for gtfs.network.py to expand coverage to 100%

Note: travis is failing in py27 and 35 for this PR due to issue with Pandana and Numpy

…xisting_stop_times_int are used, remove ValueError that can never happen
…e and unique_trip_id records to aid in debugging
… and fix rare case where index was removed before merge
… accept new GTFS modes 11 and 12, validate inputs, update docstring, add prints
…dd calendar and calendar_dates as optional tables
…dd calendar and calendar_dates as optional tables
Copy link
Member

@smmaurer smmaurer left a comment

Choose a reason for hiding this comment

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

Looks like a great refactor! Code looks good, and tests pass on my Mac.

@smmaurer
Copy link
Member

(We talked offline about the Travis failures in py27 and py35. The immediate problems are upstream, and will be fixed either in PR #87 or in a subsequent one moving the tests from Travis to GitHub Actions.)

@smmaurer smmaurer merged commit 531c394 into dev Apr 29, 2021
@smmaurer smmaurer deleted the maintenance/gtfs-network-refactor-w-unit-tests branch April 29, 2021 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants