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

Route agency_id is optional #31

Closed
rorlic opened this issue Feb 9, 2016 · 5 comments
Closed

Route agency_id is optional #31

rorlic opened this issue Feb 9, 2016 · 5 comments

Comments

@rorlic
Copy link

rorlic commented Feb 9, 2016

Class GTFS.Entities.Route (incorrectly) defines AgencyId as [Required] but the GTFS reference claims it is optional. In addition, the GTFsFeedValidation class checks the existence of the agency_id value in the list of known agencies. Obviously, if AgencyId is null, the check will fail.

            // check routes.
            var routeIds = new HashSet<string>();
            foreach(var route in feed.Routes)
            {
                if (routeIds.Contains(route.Id))
                { // oeps, duplicate id.
                    messages = string.Format("Duplicate route id found: {0}", route.Id);
                    return false;
                }
                routeIds.Add(route.Id);
                if (!agencyIds.Contains(route.AgencyId))
                {// oeps, unknown id.
                    messages = string.Format("Unknown agency found in route {0}: {1}", route.Id, route.AgencyId);
                    return false;
                }
            }
@rorlic
Copy link
Author

rorlic commented Feb 10, 2016

I have forked to rorlic/GTFS and fixed it.

rorlic pushed a commit to rorlic/GTFS that referenced this issue Feb 10, 2016
xivk added a commit that referenced this issue Feb 17, 2016
@scottharwell
Copy link

@rorlic Have you submitted your fix as a pull request to this repo? It would be nice to merge into the main repo so that NuGet subscribers can take advantage of the update automatically. I am currently experiencing a similar issue, where a feed omits this field and the GTFS library throws an exception rather than ignoring an optional field.

@xivk
Copy link
Contributor

xivk commented Feb 19, 2016

@scottharwell The fixes are already merged. The publish failed because the version # was left unchanged. There should now be a new version on nuget.

@xivk xivk closed this as completed Feb 19, 2016
@andyf101
Copy link

andyf101 commented Jun 6, 2016

GTFS.Entities.Route still has a Required attribute on AgencyId in this main repo as well as the fork. The spec shows that agency_id is optional for Route and also optional for Agency.

xivk added a commit that referenced this issue Jun 7, 2016
@xivk
Copy link
Contributor

xivk commented Jun 7, 2016

Removed the required attribute. This doesn't change anything, the attributes should be removed altogether in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants