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

Release times #254

Merged
merged 25 commits into from
Jul 4, 2023
Merged

Release times #254

merged 25 commits into from
Jul 4, 2023

Conversation

leonlan
Copy link
Member

@leonlan leonlan commented Jun 27, 2023

This PR:

Notes:

  • It is essential that you add a test when making code changes.
    This keeps the code coverage level up, and helps ensure the changes work as intended.
  • When fixing a bug, you must add a test that would produce the bug in the master branch, and then show that it is fixed with the new code.
  • New code additions must be well formatted. Changes should pass the pre-commit workflow, which you can set up locally using pre-commit.
  • Docstring additions must render correctly, including escapes and LaTeX.

@codecov
Copy link

codecov bot commented Jun 27, 2023

Codecov Report

Patch coverage: 95.00% and project coverage change: +0.02 🎉

Comparison is base (7533246) 93.98% compared to head (00d3d2d) 94.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #254      +/-   ##
==========================================
+ Coverage   93.98%   94.00%   +0.02%     
==========================================
  Files          63       63              
  Lines        2460     2469       +9     
  Branches      214      215       +1     
==========================================
+ Hits         2312     2321       +9     
+ Misses         49       48       -1     
- Partials       99      100       +1     
Impacted Files Coverage Δ
pyvrp/Model.py 100.00% <ø> (ø)
pyvrp/cpp/ProblemData.h 100.00% <ø> (ø)
pyvrp/cpp/Solution.h 100.00% <ø> (ø)
pyvrp/cpp/Solution.cpp 91.20% <75.00%> (-0.42%) ⬇️
pyvrp/cpp/ProblemData.cpp 88.09% <100.00%> (+0.29%) ⬆️
pyvrp/cpp/TimeWindowSegment.h 100.00% <100.00%> (ø)
pyvrp/cpp/search/LocalSearch.cpp 89.36% <100.00%> (+0.04%) ⬆️
pyvrp/plotting/plot_route_schedule.py 96.10% <100.00%> (+1.04%) ⬆️
pyvrp/read.py 95.06% <100.00%> (+0.32%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

pyvrp/read.py Outdated Show resolved Hide resolved
@leonlan

This comment was marked as outdated.

pyvrp/Model.py Outdated Show resolved Hide resolved
pyvrp/_Solution.pyi Outdated Show resolved Hide resolved
@leonlan leonlan changed the title Support for release times Release times Jul 3, 2023
@leonlan leonlan marked this pull request as ready for review July 3, 2023 08:46
@leonlan leonlan requested a review from N-Wouda July 3, 2023 08:47
@leonlan
Copy link
Member Author

leonlan commented Jul 3, 2023

Running a benchmark on VRPTW instances:

This branch 773db8c:

mean: 333349 (min: 333241, max: 333475)
mean gap: 0.50%

main 7533246:

mean: 333373 (min: 333212, max: 333496)
mean gap: 0.50%

Hardly any difference, so that's good news.

Copy link
Member

@N-Wouda N-Wouda left a comment

Choose a reason for hiding this comment

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

Looks good to me. A few small comments, but nothing substantial.

pyvrp/Model.py Outdated Show resolved Hide resolved
pyvrp/cpp/Solution.cpp Outdated Show resolved Hide resolved
pyvrp/cpp/TimeWindowSegment.h Outdated Show resolved Hide resolved
pyvrp/cpp/TimeWindowSegment_bindings.cpp Outdated Show resolved Hide resolved
pyvrp/_TimeWindowSegment.pyi Outdated Show resolved Hide resolved
pyvrp/tests/test_Solution.py Outdated Show resolved Hide resolved
pyvrp/tests/test_Solution.py Outdated Show resolved Hide resolved
@leonlan leonlan requested a review from N-Wouda July 4, 2023 07:57
@leonlan
Copy link
Member Author

leonlan commented Jul 4, 2023

@N-Wouda I've implemented your comments. Performance is also OK (see #254 (comment)).

Copy link
Member

@N-Wouda N-Wouda left a comment

Choose a reason for hiding this comment

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

LGTM!

@leonlan leonlan merged commit 51d9da6 into main Jul 4, 2023
11 checks passed
@leonlan leonlan deleted the release-times branch July 4, 2023 08:05
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.

Bring back support for release times
2 participants