-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat: Implement Origin/Destination matrix #301
feat: Implement Origin/Destination matrix #301
Conversation
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.
Thank you for the excellent documentation that makes this new feature easy to read. The code is also very cleaned
I just wonder if some variable names would not be better if they could be more explicit. I commented a few things about that, feel free to ignore if it adds too much noise.
Overall high quality code, it needs to be challenged by the users. Next step is implementing an interface and optimizing
PS: the test unit is very good 🥇
a99af71
to
03ae4cb
Compare
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.
Very nice work!
Description
(I'll fix the commit message after reviews.)
This is the first useful version of this feature. There is still some room for optimization, but it seems quite fast so far (2s for the medium example). Note that some trainruns don't have any endNode on this example.
Issues
close #269
Checklist
documentation/