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

inconsistence between traci and libsumo for vehicle.getLeader #7013

Closed
namdre opened this issue May 12, 2020 · 5 comments
Closed

inconsistence between traci and libsumo for vehicle.getLeader #7013

namdre opened this issue May 12, 2020 · 5 comments

Comments

@namdre
Copy link
Contributor

namdre commented May 12, 2020

if there is no leader, traci returns None whereas libsumoe always returns ("", -1)

@namdre
Copy link
Contributor Author

namdre commented May 12, 2020

@behrisch I think traci should also return ("", -1), what do you think?

@behrisch
Copy link
Contributor

None is more self explanatory but the libsumo solution is better for all the people who do automated unpacking. The problem is that such an API change can easily go unnoticed if you just check the boolean value of the result, so if leader will now always be True. So I would stay with our premise that "when in doubt, traci is right" and change libsumo instead (unless we find a way to make the API change noticeable).

@namdre
Copy link
Contributor Author

namdre commented May 13, 2020

Generally, I agree with our rule but in this case I would be in favor of braking it (for automatic unpacking and general consistency with the internal libsumo API).
Also, getLeader is currently the only method that returns 'None' as an error signal which does not strike me as a pattern to be maintained.
I'm rather confident that most users of getLeader check the leader or the distance and would notice pretty quickly that something is amiss.

@behrisch
Copy link
Contributor

OK, you are right, currently we never return None except for this one function.

@namdre namdre closed this as completed in d299cdf May 14, 2020
namdre added a commit that referenced this issue May 14, 2020
@namdre
Copy link
Contributor Author

namdre commented May 17, 2020

After seeing that several tests including simpla break, I think a more gradual approach is needed.
I will add a function to switch on the new and consistent behavior and announce that the old behavior will be deprecated eventually.

@namdre namdre reopened this May 17, 2020
@namdre namdre closed this as completed in 64426a1 May 17, 2020
namdre added a commit that referenced this issue May 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants