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

Remove the "Conditionally Forbidden" presence condition from the spec #347

Closed
isabelle-dr opened this issue Aug 22, 2022 · 3 comments
Closed
Labels
GTFS Schedule Issues and Pull Requests that focus on GTFS Schedule

Comments

@isabelle-dr
Copy link
Collaborator

isabelle-dr commented Aug 22, 2022

Context

When a field is described as Conditionally Required in the spec, three cases exist:

  1. A field or file is required under a certain condition, optional otherwise. It is the case for the calendar.txt, calendar_dates.txt, and levels.txt files, for the fields stops.stop_name, stops.stop_lat, stops.stop_lon, stops.zone_id and for many other fields.

  2. A field or file is required under a certain condition, forbidden otherwise. it is the case for fare_transfer_rules.duration_limit_type, translations.record_id, and translations.field_value.

  3. A field or file is required under a certain condition, forbidden under another condition, and optional otherwise. It is the case for stops.parent_station.

Problem

In the Fares v2 base implementation PR, a new presence condition was added to the spec: Conditionally Forbidden, and was applied to fare_transfer_rules.transfer_count. This field has the behavior described in category 2 above: required under a certain condition, forbidden otherwise.
We believe this field was initially designed as: forbidden under a certain condition, optional otherwise, and then changed when modifications were made to the PR.
This adds unnecessary complexity to the spec, this condition should only be used if a field is either forbidden or optional.

Solution

Remove the Conditionally Forbidden presence condition from the spec and describe fare_transfer_rules.transfer_count as Conditionally Required.

I'm happy to open a PR to do this fix if there is agreement from the community.

@isabelle-dr isabelle-dr added the GTFS Schedule Issues and Pull Requests that focus on GTFS Schedule label Nov 4, 2022
@emmambd
Copy link
Collaborator

emmambd commented May 10, 2023

👋 I'm going through our issue backlog to see if there's anything relevant to roll into the scope for #376. It looks like #376 may be a good opportunity to remove Conditionally Forbidden as well since there was 4 thumbs up in favor of this issue and no dissent.

Checking in with @isabelle-dr and those who +1'd: @westontrillium, @botanize, @timMillet, @jfabi

@emmambd emmambd added the Status: Ready for Pull Request Issues that are ready to be transferred to the Pull Request stage. label May 10, 2023
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It may be closed manually after one month of inactivity. Thank you for your contributions.

@github-actions github-actions bot added the stale label May 10, 2024
@isabelle-dr
Copy link
Collaborator Author

We can close - we can now make use of Conditionally Forbidden

@eliasmbd eliasmbd removed the Status: Ready for Pull Request Issues that are ready to be transferred to the Pull Request stage. label May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GTFS Schedule Issues and Pull Requests that focus on GTFS Schedule
Projects
None yet
Development

No branches or pull requests

3 participants