jamespfennell / gtfs Goto Github PK

I recently started looking into adding vehicle positions to Transiter, and it seems for that this package also needs to be updated to fully parse the Vehicle real time entity type. My first thought was to modify the existing gtfs.Vehicle type to something like this:

type VehiclePosition = gtfsrt.VehiclePosition

type Vehicle struct {
	ID *VehicleID

	Trip *Trip

	Position *VehiclePosition

	IsEntityInMessage bool
}

This adds some redundancies to the existing structure, since VehicleID already contains some information in VehiclePosition. If backwards compatibility isn't a concern, then the structure could be further modified to match gtfsrt.VehiclePosition.

After the data contract is settled, I think this should be a relatively simple change (famous last words haha) but I wanted to get your initial impressions on the above data structure change as well as any other thoughts you may have on the larger idea of implementing vehicles positions end-to-end.

Throughout this library we print warnings to stdout whenever there is something unexpected like a missing trip ID. Logging like this is not great because this is a library, and applications that use it may have their own logging setup. For example Transiter has structured logging and a way of disabling all logging, but that doesn't disable logs here.

I think a nice change would be to remove the logging, and instead could return a slice of Warning values in the Static struct. Each warning would contain the type of warning, the file name, the line number and some additional metadata. Maybe it could be like this:

type Warning struct {
  // The kind of warning. (We avoid using the word type because this collides with the type keyword.)
  kind        WarningKind
  // The file this warning came from e.g. stops.txt.
  file        string
  // The line number within the file of the problematic row.
  lineNumber  int
  // Within the line, the cell that is the problem.
  cellNumber  int
  // The full content of the problematic row
  row         string
  // Labels relevant to the warning.
  // For example, for invalid shape ID we could have the shape ID here.
  labels      map[string]string
}

type WarningKind int
const (
  MissingTripID WarningKind = iota
  InvalidShapeID
  // etc
)

When I parse the most recent GTFS realtime feed for the NYC L train I see many duplicate vehicles in the result. One vehicle has IsEntityInMessage: false and the other has IsEntityInMessage: true. Presumably the parser sees the first vehicle attached to a TripUpdate and the second attached to a VehiclePosition. It should de-duplicate these - in fact, the IsEntityInMessage: false vehicle should just be dropped in this case.

Observed after jamespfennell/transiter#110 was submitted to Transiter mainline. In Transiter I see lots of debug logging warning of duplicate vehicles in the NYC subway feed, and I think the root cause is here.

CC @cedarbaum.

The GTFS spec defines optional columns like so:

Optional - The field may be omitted from the dataset. If an optional column is included, some of the entries in that field may be empty strings. To enter an empty string, just omit any text between the commas for that field. Note that an omitted field is equivalent to a field that is entirely empty.

In the package we often incorrectly distinguish between "column not provided" and "column is not empty". For example, Agency.Language is a *string field that is <nil> if the column is missing but &"" if the column is empty. But these should have the same result.

For efficiency and ergonomics it's likely best to convert optional string columns to "".

I really like this package, as it is an easy to use and well performing abstraction to work with GTFS, however, a sizeable amount of files from the standard are still missing.

Specifically, the following required files don't seem to be currently supported:

• stop_times
• calendar
• calendar_dates

It would also be nice to have support for optional files, such as

• shapes

Described here: https://developers.google.com/transit/gtfs/reference/extended-route-types