-
Notifications
You must be signed in to change notification settings - Fork 126
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
Jormungandr: fix reverse isochrons #1082
Conversation
} else { | ||
ep = destinations[0]; | ||
} | ||
return navitia::routing::make_isochrone(*planner, ep, request.datetimes(0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you need to specify if ep
is the origin or the destination?
The direction is fixed by the from/to param in isochrone: from -> clockwise, to -> non-clockwise. Jormungandr should do something about that and kraken or raptor should check the coherence of these params. |
you're right I added a test on this |
add tests and then fix the discovered bugs :)
normal_response, error_code = self.query_no_assert(q) | ||
|
||
assert error_code == 404 | ||
assert normal_response['error']['message'] == 'isochrone works only for clockwise request' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
linefeed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
OK for me |
and return a nice error when no solution found
Fine, but you need to update the documentation of isochrone |
Jormungandr: fix reverse isochrons
🖕 |
@stifoon manage your team! |
isochrons were working only with a 'from'argument, they now also works with only a 'to' argument.
fix http://jira.canaltp.fr/browse/NAVITIAII-1774
@yohanboniface, it should fix your problem, thanks for the bug report!
fixes SNCFdevelopers/API-trains-sncf#7