search

afimb / gtfslib-python

An open source library in python for reading GTFS files and computing various stats and indicators about Public Transport networks
GNU General Public License v3.0
44 stars 6 forks source link

Handle new transfers fields #68

Closed pailakka closed 2 years ago

pailakka commented 5 years ago

Handle from_trip_id and to_trip_id fields in transfers.txt

laurentg commented 5 years ago

Travis CI fails:

../home/travis/virtualenv/python2.7.14/lib/python2.7/site-packages/sqlalchemy/sql/crud.py:793: SAWarning: Column 'transfers.from_trip_id' is marked as a member of the primary key for table 'transfers', but has no Python-side or server-side default generator indicated, nor does it indicate 'autoincrement=True' or 'nullable=True', and no explicit value is passed. Primary key columns typically may not store NULL. Note that as of SQLAlchemy 1.1, 'autoincrement=True' must be indicated explicitly for composite (e.g. multicolumn) primary keys if AUTO_INCREMENT/SERIAL/IDENTITY behavior is expected for one of the columns in the primary key. CREATE TABLE statements are impacted by this change as well on most backends.

pailakka commented 5 years ago

Thanks for your feedback! I realized that trip_ids in transfers.txt make transfers rather hard to model in a DB as to/from_trip_id is optional field and there could be many combinations of trip_ids for every pair of stop ids. To handle this, I added transfer_id autoincrement field to act as primary key. I'm not however sure if this is good enough solution?

laurentg commented 5 years ago

Great, thanks! Indeed I'm not a specialist too, having to define a multi-column (4 fields!) primary key with both non-null and null values seems a bit complex. I do not see any drawback on having an auto-increment ID column.

I assume you have tested your changes? I can merge it w/o testing myself. Would it be possible to include a few unit tests with transfer trip IDs?

pailakka commented 5 years ago

I'll do some more test and implement a few unit tests