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

Rule change: make stop_without_zone_id conditional on fare rule type #1663

Closed
emmambd opened this issue Feb 1, 2024 · 4 comments · Fixed by #1693
Closed

Rule change: make stop_without_zone_id conditional on fare rule type #1663

emmambd opened this issue Feb 1, 2024 · 4 comments · Fixed by #1693
Labels
enhancement New feature request or improvement on an existing feature status: Ready An issue that is ready to be worked on.

Comments

@emmambd
Copy link
Contributor

emmambd commented Feb 1, 2024

Describe the problem

As highlighted by Trillium on the spec repository, right now stop_without_zone_id's logic is that if fare_rules.txt contains records, then presence of zone_id is required for all stops in the dataset.

However, zone_id should not be required for route_based fares.

Proposed solution

Update logic so notice is triggered IF 1 stop without a zone_id has a trip_id in stop_times.txt whose route_id defines origin_id, destination_id OR contains_id in fare_rules.txt.

Alternatives you've considered

No response

Additional context

No response

@emmambd emmambd added enhancement New feature request or improvement on an existing feature status: Needs triage Applied to all new issues labels Feb 1, 2024
@emmambd emmambd added this to the Now milestone Feb 1, 2024
@emmambd emmambd added status: Ready An issue that is ready to be worked on. and removed status: Needs triage Applied to all new issues labels Feb 1, 2024
@krnlmstrd
Copy link

I'm wondering about the case where a stop:

  1. does not have a zone_id defined
  2. belongs to multiple routes
  3. some of these routes are included in fare_rules with only fare_id and route_id defined (i.e. valid)
  4. some of these routes are not included in fare_rules

In this case, it doesn't seem right to warn with stop_without_zone_id because the feed producer's fix is to include the missing routes in fare_rules, not to define a zone_id for the stop.

I also wonder if stop_without_zone_id is the correct warning for a stop without a zone_id and with no routes included in fare_rules, because in the same way the fix might not be to define a zone_id.

Could this be clarified with three different warnings?

  1. Fare Rule Has Conflicting Definitions Notice
    • when a fare rule is not mutually exclusive in defining (route_id) or (origin_id, destination_id, contains_id)
    • the spec is not explicit that these fields are conditionally required/forbidden, although it seems implicit based on my reading of Weston's example
    • Is there a case where a fare rule could validly have a route and origin etc. defined? For example, an express route and a local route share tracks, and fares are calculated on both the route taken and the zones of travel.
  2. Stop Without Complete Fare Rule Notice
    • when fare_rules are defined and a stop has neither (a zone_id defined) nor (all routes included in fare_rules)
  3. Stop With Conflicting Fare Rule Notice
    • when fare_rules are defined and a stop has both (a zone_id defined) and (some routes included in fare_rules)

@emmambd
Copy link
Contributor Author

emmambd commented Feb 27, 2024

Hi @krnlmstrd — thanks for your patience waiting for a reply. We did a deep dive internally here to consider your proposal. I think there a few things to clarify here:

In what situations does a feed producer see the stop_without_zone_id error?

With the proposed revision above, the feed producer will only see this error when they've used zone-based fare related fields within their fares_rules.txt file: Update logic so notice is triggered IF 1 stop without a zone_id has a trip_id in stop_times.txt whose route_id defines origin_id, destination_id OR contains_id in fare_rules.txt.

That means anyone relying on route-based fare rules (even partially) won't see this rule with the update, which I believe addresses your initial concerns.

Should the validator suggest/infer a producer make changes if their fare rules are incomplete?

Your other suggestion is to have some kind of warning notice that flags to a producer when they have routes that are missing fare rules. This will fall in our INFO category of notices, where a feed doesn't explicitly violate the spec's MUSTs (error) or SHOULDs (warning), but some issue is present that looks suspicious and should be reviewed by the producer. After some consideration, we didn't think we can safely assume that a producer typically means to add fare rules to all their routes or stops, and so having an INFO notice seemed excessive. I'd be curious if you have some use cases to highlight situations where this has come up.

Do route-based and zone-based rules conflict?

Based on the spec, there is no violation in a situation where a stop belongs to both a zone-based fare rule and a route defined in a route-based fare rule.

@krnlmstrd
Copy link

Thanks for the clarification! That answers my questions about how to fix this particular issue.

I wasn't thinking of any particular use cases. I assumed that, if fare_rules are defined and at least one stop is not associated with a fare rule (via zone or route), it is more likely a mistake than an intentional omission. But I don't know how producers tend to use the fare-related tables, so avoiding unnecessary notices makes sense.

I am curious what it means when a stop has a zone defined in a fare rule and the stop belongs to a route defined in a different fare rule - but I think that's because I need to look into how fare rule logic works.

@emmambd
Copy link
Contributor Author

emmambd commented Feb 28, 2024

@krnlmstrd Thanks Michael! Let us know if you want to discuss anything else to help clarify things.

One note: we're aiming to complete all work for the upcoming release by March 8th, so we have enough time to run feed analysis on the changes and identify bugs before we release it March 21st. If you're aiming to pick up this issue, knowing your rough timeline would be helpful.

krnlmstrd pushed a commit to michaelandrewkearney/gtfs-validator that referenced this issue Mar 1, 2024
…yData#1663)

- Update `StopZoneIdValidator` to issue notice about a stop without `zone_id` defined only when the stop is contained in a trip contained in a route defined in a fare rule with zone fields defined.
- Change from previous logic which warned about stops without `zone_id` defined if any fare rules had zone fields defined.
- The warning is still only triggered for stops of location type `0`.
- Zone fields in `fare_rules.txt` are `origin_id`, `destination_id`, and `contains_id`.
krnlmstrd pushed a commit to michaelandrewkearney/gtfs-validator that referenced this issue Mar 7, 2024
…yData#1663)

- Update `StopZoneIdValidator` to issue notice about a stop without `zone_id` defined only when the stop is contained in a trip contained in a route defined in a fare rule with zone fields defined.
- Change from previous logic which warned about stops without `zone_id` defined if any fare rules had zone fields defined.
- The warning is still only triggered for stops of location type `0`.
- Zone fields in `fare_rules.txt` are `origin_id`, `destination_id`, and `contains_id`.
@emmambd emmambd removed this from the Now milestone Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request or improvement on an existing feature status: Ready An issue that is ready to be worked on.
Projects
None yet
2 participants