public-transport / friendly-public-transport-format

A format for APIs, libraries and datasets containing and working with public transport data.
Creative Commons Attribution Share Alike 4.0 International
122 stars 1 forks source link

canceled trips & departures/arrivals #27

Open derhuerst opened 6 years ago

derhuerst commented 6 years ago

What should we do with them? Omit them from the results? Mark them as canceled?

see also derhuerst/vbb-rest#19

kiliankoe commented 6 years ago

Omitting elements doesn't feel like the right approach. The goal should be to be as comprehensible instead of as minimal as possible imo.

A cancelled state seems like a good idea.

derhuerst commented 6 years ago

I'm in favour of marking them as canceled. But I also think that's not enough. Just like with realtime departures, we should incentivise people to handle canceled departures properly. We could additionally set departure/when to null.

derhuerst commented 6 years ago

Also, we need to pay attention to the spelling of cancel(l)ed.

derhuerst commented 6 years ago

from https://github.com/derhuerst/vbb-rest/issues/19#issuecomment-350275017

IMHO: I think that is an important information for everyone. One should be able to see it at a glance. I strongly suggest to show the key all the time.

juliuste commented 6 years ago

Cancel(l)ed state LGTM, I'm not sure if it would be wise to set the departure/when to null though, since that would take away useful information (e.g. if you want to know which train (at what time) actually had been cancel(l)ed).

derhuerst commented 6 years ago

From derhuerst/vbb-rest#26:

It seems, that the when field is null when a trip has been canceled. If it is possible, it would be nice, if you'd provide the original time even if a trip is cancelled.

It would be easier in many situations to identify the cancelled trip.

I'm sorting the departures by time in my MagicMirror module and I really would like to show the cancelled trips in the module to inform the user about cancelled trips instead of removing them from the departures list (because there's no logical position for them if they have no departure time)...

deg0nz commented 6 years ago

I must say, that I really like the canceled flag. But as stated in the comment above, I have some problems with nulling the when field when a trip has been canceled.

Removing this time value forces me to sort these departures out entirely.

Also, please consider the behaviour of the "official" applications. When you use public transport apps, the canceled trips are still shown with their original departure times, but they provide a hint or the departures are crossed out (like BVG, DB apps do). So even if the trip is canceled, the information still exists. IMHO the API should behave in a similar way. So I agree with @juliuste's comment above (https://github.com/public-transport/friendly-public-transport-format/issues/27#issuecomment-350715482)

But I also agree with @derhuerst when he says, that people should be incentivised to handle canceled departures properly.

Maybe you could null the when field per default and provide a flag on the request to provide the departure time in the response. This way the requestor is actually aware what data he'll be receiving. This could satisfy both sides.

But either way, I really think this data should be accessible somehow.

derhuerst commented 6 years ago

What about this?

{
  when: null,
  canceled: true,
  originalWhen: '2018-02-28T22:19:25.066Z'
}

pros:

cons:

deg0nz commented 6 years ago

This could be a good solution. Do you want to provide the originalWhen field all the time or only on canceled trips?

If you provide it all the time you should consider providing it without the delay delta.

derhuerst commented 6 years ago

Not sure yet. I think having it all the time would be confusing. Or at least we would need a better name than "original", because "original" might also be understood as "scheduled" (vs. realtime).

kiliankoe commented 6 years ago

one could make it a nested object getting more clarity in a complexity tradeoff :/

{
    when: null,
    status: {
        status: 'cancelled',
        originalWhen: '...'
    }
}

I would however also vote for a naming change instead of this monstrosity.

derhuerst commented 6 years ago

I propose a field formerWhen (see also former vs. previous), which contains the former realtime value.

deg0nz commented 6 years ago

I would also suggest to name the field something like scheduledWhen. But maybe formerWhen makes it more clear that we are dealing with a canceled departure. So I would agree to go with formerWhen.

derhuerst commented 6 years ago

Note that scheduledWhen is something different. A departure can be scheduled (aka planned) or prognosed (aka realtime). Additionally, it may have been canceled, which is why I picked "former" instead of "scheduled".

deg0nz commented 6 years ago

I'm aware of that, but as I understand the meaning of the word "scheduled" (when I translate it to german with "geplant"/"planmÀßig"), a departure is still scheduled to a certain time even if it is canceled or delayed. Because it was originally scheduled to leave at this specific time. IMHO, the cancellation itself doesn't change anything to the original scheduled time, because the schedule itself is still valid, but with a cancelled departure.

Even the delay doesn't change anything about the actual scheduled time. It just changes the actual departure time.

But this is just my opinion...

If I think about it that way, one could change the response to something like that:

On a normal departure:

{
    when: <SCHEDULEDTIMESTAMP+120>,
    delay: 120,
    scheduled: <SCHEDULEDTIMESTAMP>
}

On a cancelled departure:

{
    when: null,
    delay: null,    // Or nonexistent for less data
    scheduled: <SCHEDULEDTIMESTAMP>,
    canceled: true
}

pros:

cons:

As I said, this is just my opinion on this. I would also be fine with a formerWhen field. :)

Edit: It could also be the case, that I'm missing something here...

derhuerst commented 6 years ago

I'm aware of that, but as I understand the meaning of the word "scheduled" (when I translate it to german with "geplant"/"planmÀßig"), a departure is still scheduled to a certain time even if it is canceled or delayed. Because it was originally scheduled to leave at this specific time. IMHO, the cancellation itself doesn't change anything to the original scheduled time, beacuse the schedule itself is still valid, but with a cancelled departure.

Apparently there's been a misunderstanding. I trying to make the point that "scheduled" wouldn't be the correct name for the former when, because the former when has been a realtime value (not a scheduled point in time).

But, thinking about it, it might make sense to, if a departure has been cancelled, expose the scheduled departure, because the former realtime departure is irrelevant anyways. πŸ‘

So, my new proposal:

{
  when: null,
  canceled: true,
  formerScheduledWhen: '...'
}
deg0nz commented 6 years ago

Ok, then I really didn't get your point ^^'

This sounds good to me. I'd say formerScheduledWhen would work fine.

juliuste commented 6 years ago

(Sorry that it took me so long to respond)

What about delay information when a trip is canceled? IMHO that information should be kept as well. Would make more sense to call it formerWhen + delay then again, though πŸ˜„

And (different topic) from what I understand, the canceled field should be required for every journey to make things clearer, right?

derhuerst commented 6 years ago

What about delay information when a trip is canceled? IMHO that information should be kept as well. Would make more sense to call it formerWhen + delay then again, though πŸ˜„

Not sure if the former delay is that relevant. After all, the departure/trip has been cancelled.

(I know arguing with what the status quo is like is not a very strong argument, but) Whenever I see cancelled departures in current signage systems, the former delay isn't being shown anymore.

And (different topic) from what I understand, the canceled field should be required for every journey to make things clearer, right?

I'm fine with that.

juliuste commented 6 years ago

Not sure if the former delay is that relevant. After all, the departure/trip has been cancelled.

For example when recording which trains have been canceled, there might be a correlation with the "former" delay (Trains that are really late might be more likely to be canceled) which IMHO makes this a relevant information.

derhuerst commented 6 years ago

I consider this a niche use case. Because we may not even have this information, I'm against adding formerDelay as a required field.

juliuste commented 6 years ago

I consider this a niche use case. Because we may not even have this information, I'm against adding formerDelay as a required field.

I never wanted the field to be required πŸ˜„ I agree that one might not have this information, after all even the delay information on regular journeys isn't required right now. But why not make it an optional field?

derhuerst commented 6 years ago

Okay, then I misunderstood you.

I'm fine with making it optional. I'm just not sure if it's necessary. Shall I add it to #31?

juliuste commented 6 years ago

Please note that instead of formerScheduledWhen we would have formerWhen (+ optional formerDelay) then.

Shall I add it to #31?

Yes πŸ™‚

derhuerst commented 6 years ago

Please note that instead of formerScheduledWhen we would have formerWhen (+ optional formerDelay) then.

Okay, I'm against that. πŸ˜› As I said above, I think it is more relevant to expose the former scheduled departure/arrival.

That aside, I've already built it into hafas-client and deployed among the APIs, so your proposal would be a breaking change there.

juliuste commented 6 years ago

I don't get your point. Either you have delay information on the canceled route, then my proposal is not a problem (?). Or you don't have it, but calculating the raw formerScheduledWhen is also impossible then, right? Because if you have the "normal" when, you need to to scheduledWhen = when - delay to calculate it?

That aside, I've already built it into hafas-client and deployed among the APIs, so your proposal would be a breaking change there.

I don't think this decision should depend on hafas-clients implementation πŸ˜› (Especially since there's a breaking change coming with hafas-client#33 anyways πŸ˜„)

derhuerst commented 6 years ago

I don't think this decision should depend on hafas-clients implementation πŸ˜›

Even though hafas-client should not dictate choices made here, it is another argument (aside that I think it is a counter-intuitive proposal) against changing formerScheduledWhen.

To make it clear: Why not add formerDelay as an optional field?

derhuerst commented 6 years ago

[just a sarcastic side note]

Why I think having the delay on cancelled departures (by default) is counter-intuitive. πŸ˜›

https://mobile.twitter.com/BahnAnsagen/status/974380477040775170/photo/1

juliuste commented 6 years ago

I feel like this is inconsistent. We should either use scheduledWhen + delay in non-canceled routes as well, or use formerWhen and delay in canceled routes. Why use the scheduled keyword here and not everywhere?

deg0nz commented 6 years ago

@juliuste Has a point here...

juliuste commented 6 years ago

@derhuerst and I just had a long discussion about this, I tried my best to summarize our proposal in #33

Since #33 would introduce a breaking change, I'm fine if @derhuerst wants to add his current proposal (formerScheduledWhen) to fptf@1 since modules like hafas-client apparently already work this way.

For fptf@2 #33 seems to be a more elegant solution, though πŸ™‚

timaschew commented 6 years ago

a bit related to https://github.com/public-transport/friendly-public-transport-format/issues/44