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

canceled trips & departures/arrivals #27

Open
Tracked by #9
derhuerst opened this issue Dec 5, 2017 · 32 comments
Open
Tracked by #9

canceled trips & departures/arrivals #27

derhuerst opened this issue Dec 5, 2017 · 32 comments
Milestone

Comments

@derhuerst
Copy link
Member

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

see also derhuerst/vbb-rest#19

@kiliankoe
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

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

@derhuerst
Copy link
Member Author

derhuerst commented Dec 8, 2017

from derhuerst/vbb-rest#19 (comment)

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
Copy link
Member

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
Copy link
Member Author

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
Copy link

deg0nz commented Feb 28, 2018

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 (#27 (comment))

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
Copy link
Member Author

What about this?

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

pros:

  • prevents people from handling the data properly
  • allows people to show the time of the canceled dep

cons:

  • yet another field in the spec

@deg0nz
Copy link

deg0nz commented Feb 28, 2018

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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member Author

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

@deg0nz
Copy link

deg0nz commented Mar 6, 2018

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
Copy link
Member Author

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
Copy link

deg0nz commented Mar 7, 2018

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:

  • Maybe this approach could clarify the different meanings of the different times without any more explanation to the format itself.
  • And since we are talking about departures in any case of this data, the words "scheduled" and "when" would be clear enough IMO.

cons:

  • more data on a normal request
  • still one more field in the spec

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
Copy link
Member Author

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
Copy link

deg0nz commented Mar 7, 2018

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

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

@juliuste
Copy link
Member

juliuste commented Mar 17, 2018

(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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

juliuste commented Mar 17, 2018

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
Copy link
Member Author

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
Copy link
Member

juliuste commented Mar 17, 2018

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

Shall I add it to #31?

Yes 🙂

@derhuerst
Copy link
Member Author

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
Copy link
Member

juliuste commented Mar 17, 2018

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
Copy link
Member Author

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
Copy link
Member Author

[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
Copy link
Member

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
Copy link

deg0nz commented Mar 17, 2018

@juliuste Has a point here...

@juliuste
Copy link
Member

@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 🙂

This was referenced Mar 22, 2018
@juliuste juliuste added this to the 2.0.0 milestone Jul 27, 2018
@timaschew
Copy link

a bit related to #44

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

Successfully merging a pull request may close this issue.

5 participants