rust-transit / gtfs-structure

Read a GTFS file
MIT License
61 stars 32 forks source link

Make (continuous) pickup drop off non-optional / allow parsing invalid values #101

Closed stefanw closed 3 years ago

stefanw commented 3 years ago

This brings PickupDropOffType and ContinuousPickupDropOff in line with other optional enums with default values like LocationType. The GTFS specification makes (continuous) pickup/drop off optional, but assigns a default value on empty, so the Option type is misleading: e.g. None for pickup_type means the same as Some(PickupDropOffType::Regular).

That's why I removed the Options on pickup_type, drop_off_type, continuous_pickup and continuous_drop_off. This might be a backwards incompatible change.

Additionally, this change allows parsing invalid values for the (continuous) pickup / drop off columns (e.g. "-999", hello again MTA Brooklyn GTFS) and returning the default value instead.

Tristramg commented 3 years ago

I suppose at some point we’ll need a strict flag to decide the behaviour in case of invalid values.

For now I agree with your pull request, thank you!

Could you just please bump the version, as this is breaking the API?

antoine-de commented 3 years ago

Hello,

As this would be a bit too permissive, and the behavior might be different for two transit agencies, could it possible to add an Unknown(String) to the enum? "" would map to Regular, the MTA file would parse, but you'd have to handle it by hand. This would also allow to manage some extensions.

If you don't have enough time, we can make that change for you.

Tristramg commented 3 years ago

I’m merging it and I’ll work from there.

Thank you for the contribution!