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.

Sign up for GitHub

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

Jump to bottom

Visualize results in geojson #165

Merged
merged 10 commits into from
Apr 9, 2021
Merged

Visualize results in geojson #165

merged 10 commits into from
Apr 9, 2021

Conversation

fonsecadeline
Copy link
Contributor

No description provided.

@fonsecadeline
Copy link
Contributor Author

This PR requires #153

@fonsecadeline
Copy link
Contributor Author

fonsecadeline commented Mar 25, 2021 •
edited
Loading

  • Add test
  • Validate with GithubActions

@fonsecadeline fonsecadeline force-pushed the visualize_clusters branch 7 times, most recently from edb0108 to cf0b1f9 Compare March 26, 2021 08:01
@fonsecadeline
Copy link
Contributor Author

fonsecadeline commented Mar 26, 2021 •
edited
Loading

This is only a first version, I will work on merging OutputHelper::Clustering and OutputHelper::Result functions (collect_hulls mainly). Main issue is that we do not provide data with the same form.

@fonsecadeline fonsecadeline force-pushed the visualize_clusters branch 5 times, most recently from 5d11bf8 to b0d73bc Compare March 26, 2021 10:47
braktar
braktar suggested changes Mar 29, 2021
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
api/v01/entities/vrp_input.rb Outdated Show resolved Hide resolved
api/v01/entities/vrp_input.rb Show resolved Hide resolved
api/v01/entities/vrp_result.rb Show resolved Hide resolved
lib/output_helper.rb Outdated Show resolved Hide resolved
api/v01/entities/vrp_result.rb Show resolved Hide resolved
braktar
braktar suggested changes Mar 29, 2021
View reviewed changes
api/v01/vrp.rb Outdated Show resolved Hide resolved
optimizer_wrapper.rb Outdated Show resolved Hide resolved
wrappers/ortools.rb Outdated Show resolved Hide resolved
wrappers/ortools.rb Outdated Show resolved Hide resolved
@fonsecadeline fonsecadeline force-pushed the visualize_clusters branch 2 times, most recently from bd68ef1 to f6c8ab0 Compare March 29, 2021 12:26
@fonsecadeline fonsecadeline force-pushed the visualize_clusters branch 3 times, most recently from 1bce603 to 0361e13 Compare March 30, 2021 10:02
senhalil
senhalil suggested changes Mar 30, 2021
View reviewed changes
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.

Phew, this was a long one 😅 thanks @fonsecadeline !

My main comment is that we can make the generate_geometry function return the generated geometries and keep nothing -- like we do for csv functionality. That was the main point of doing this at the end point. That way when we make a change (improvement/extension/correction) to the geometry output/functionality, solutions that are already GET'ed/dumped can be re-GET'ed to have the latest geometry without an issue -- like the way it is possible for us to add a new column to the CSV output and new GETs would show this new column.

Otherwise, there are some small fixes to do but I think, the PR is mainly there.

api/v01/vrp.rb Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
api/v01/entities/vrp_input.rb Show resolved Hide resolved
models/types/geometry_type.rb Show resolved Hide resolved
models/types/geometry_type.rb Outdated Show resolved Hide resolved
test/models/vrp_test.rb Outdated Show resolved Hide resolved
util/job_manager.rb Outdated Show resolved Hide resolved
wrappers/ortools.rb Outdated Show resolved Hide resolved
wrappers/wrapper.rb Show resolved Hide resolved
wrappers/wrapper.rb Show resolved Hide resolved
braktar
braktar suggested changes Apr 1, 2021
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
optimizer_wrapper.rb Outdated Show resolved Hide resolved
@fonsecadeline fonsecadeline force-pushed the visualize_clusters branch 2 times, most recently from ea3c2c8 to a8fd4e3 Compare April 1, 2021 10:09
fonsecadeline
fonsecadeline commented Apr 1, 2021
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
senhalil
senhalil suggested changes Apr 2, 2021
View reviewed changes
api/v01/entities/vrp_input.rb Outdated Show resolved Hide resolved
api/v01/vrp.rb Outdated Show resolved Hide resolved
lib/output_helper.rb Outdated Show resolved Hide resolved
models/concerns/expand_data.rb Outdated Show resolved Hide resolved
models/types/geometry_type.rb Show resolved Hide resolved
models/types/geometry_type.rb Outdated Show resolved Hide resolved
test/models/vrp_test.rb Outdated Show resolved Hide resolved
wrappers/ortools.rb Outdated Show resolved Hide resolved
wrappers/wrapper.rb Show resolved Hide resolved
wrappers/wrapper.rb Show resolved Hide resolved
@fonsecadeline fonsecadeline force-pushed the visualize_clusters branch 3 times, most recently from a5d8ddf to d052c0b Compare April 2, 2021 14:25
@fab-girard fab-girard changed the title Visualize clusters Visualize results in geojson Apr 6, 2021
braktar
braktar approved these changes Apr 6, 2021
View reviewed changes
api/v01/entities/vrp_input.rb Outdated Show resolved Hide resolved
senhalil
senhalil reviewed Apr 6, 2021
View reviewed changes
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.

few questions

models/types/geometry_type.rb Outdated Show resolved Hide resolved
models/types/geometry_type.rb Outdated Show resolved Hide resolved
optimizer_wrapper.rb Outdated Show resolved Hide resolved
optimizer_wrapper.rb Outdated Show resolved Hide resolved
senhalil
senhalil approved these changes Apr 6, 2021
View reviewed changes
models/vrp.rb Outdated Show resolved Hide resolved
@fab-girard fab-girard merged commit f26f1a8 into Mapotempo:dev Apr 9, 2021
@fonsecadeline fonsecadeline deleted the visualize_clusters branch April 9, 2021 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@senhalil senhalil senhalil approved these changes

@braktar braktar braktar approved these changes

@fab-girard fab-girard Awaiting requested review from fab-girard

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fonsecadeline @braktar @senhalil @fab-girard