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

Fix split independent functions #50

Merged

Conversation

fonsecadeline
Copy link
Contributor

No description provided.

Copy link
Contributor

@senhalil senhalil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small changes

wrappers/wrapper.rb Outdated Show resolved Hide resolved
optimizer_wrapper.rb Outdated Show resolved Hide resolved
wrappers/wrapper.rb Outdated Show resolved Hide resolved
wrappers/wrapper.rb Outdated Show resolved Hide resolved
wrappers/wrapper.rb Outdated Show resolved Hide resolved
wrappers/wrapper.rb Outdated Show resolved Hide resolved
@senhalil senhalil requested review from fab-girard and removed request for braktar August 6, 2020 08:59
Copy link
Contributor

@senhalil senhalil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some issues with compact, delete_if and variable_naming

Copy link
Contributor

@senhalil senhalil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The person you have called cannot be reached at the moment, try again later :)

wrappers/wrapper.rb Outdated Show resolved Hide resolved
@fonsecadeline fonsecadeline force-pushed the fix_split_independent_functions branch 2 times, most recently from bbb22ec to a7326e9 Compare September 28, 2020 08:59
Copy link
Contributor

@senhalil senhalil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there might be a rebase conflict mistake

optimizer_wrapper.rb Outdated Show resolved Hide resolved
wrappers/wrapper.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@senhalil senhalil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@senhalil senhalil self-requested a review September 28, 2020 13:56
Copy link
Contributor

@senhalil senhalil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the PR looks okay but there are broken tests :/

Copy link
Contributor

@senhalil senhalil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgotten assert_correctness_provided_matrix_indices

wrappers/ortools.rb Outdated Show resolved Hide resolved
# Intead of map{}.compact() or collect{}.compact() reduce([]){} or each_with_object([]){} is more efficient
# when there are items to skip in the loop because it makes one pass of the array instead of two
vrp.vehicles.each_with_index.each_with_object([]) { |(vehicle, v_i), sub_vrps|
vrp.vehicles.each_with_index.collect{ |vehicle, v_i|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

map.with_index ?

@fonsecadeline fonsecadeline force-pushed the fix_split_independent_functions branch from 1201df9 to 9ca90da Compare October 6, 2020 13:49
@fab-girard fab-girard merged commit 80bab21 into Mapotempo:dev Oct 11, 2020
@fonsecadeline fonsecadeline deleted the fix_split_independent_functions branch October 12, 2020 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants