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

editoast: add get_catenaries_on_path endpoint #3716

Merged
merged 2 commits into from
Apr 18, 2023
Merged

editoast: add get_catenaries_on_path endpoint #3716

merged 2 commits into from
Apr 18, 2023

Conversation

Castavo
Copy link
Contributor

@Castavo Castavo commented Mar 29, 2023 •
edited

closes: #3605

In this PR, I add:

  • Methods on the Pathfinding model.
  • A Catenary model specifically designed for fixtures.
  • A catenaries_on_pathfinding view.
  • An empty_infra fixture in fixture.rs and a infra_with_catenaries fixture.

@codecov
Copy link

codecov bot commented Mar 29, 2023 •
edited

Codecov Report

Merging #3716 (29c28ca) into dev (246312e) will increase coverage by 0.19%.
The diff coverage is 95.86%.

@@             Coverage Diff              @@
##                dev    #3716      +/-   ##
============================================
+ Coverage     69.64%   69.83%   +0.19%     
  Complexity     2026     2026              
============================================
  Files           439      441       +2     
  Lines         22288    22432     +144     
  Branches       1672     1672              
============================================
+ Hits          15523    15666     +143     
- Misses         5960     5961       +1     
  Partials        805      805
Flag Coverage Δ
editoast 74.81% <95.04%> (+0.76%) ⬆️
front 59.34% <100.00%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
editoast/src/error.rs 35.29% <ø> (+3.92%) ⬆️
editoast/src/infra_cache/mod.rs 73.30% <ø> (+1.45%) ⬆️
editoast/src/models/mod.rs 37.03% <ø> (ø)
editoast/src/schema/mod.rs 88.33% <0.00%> (-6.31%) ⬇️
editoast/src/views/pathfinding/mod.rs 12.50% <50.00%> (ø)
editoast/src/views/pathfinding/catenaries.rs 98.85% <98.85%> (ø)
editoast/src/models/infra.rs 82.85% <100.00%> (+0.50%) ⬆️
editoast/src/models/pathfinding.rs 100.00% <100.00%> (ø)
front/src/common/api/osrdEditoastApi.ts 91.62% <100.00%> (+0.15%) ⬆️

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Castavo Castavo force-pushed the bpt/catenaries-on-path branch 4 times, most recently from 2a42c19 to 22ba6db Compare March 30, 2023 16:11
@Castavo Castavo marked this pull request as ready for review March 30, 2023 16:29
@Castavo Castavo requested a review from a team as a code owner March 30, 2023 16:29
@Castavo Castavo force-pushed the bpt/catenaries-on-path branch 2 times, most recently from 9def3e3 to ec9b2e0 Compare April 6, 2023 13:32
leovalais
leovalais previously approved these changes Apr 6, 2023
View reviewed changes
Copy link
Contributor

@leovalais leovalais left a comment

Choose a reason for hiding this comment

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

LGTM :)

editoast/src/models/pathfinding.rs Show resolved Hide resolved
editoast/src/views/pathfinding/catenaries.rs Show resolved Hide resolved
flomonster
flomonster requested changes Apr 6, 2023
View reviewed changes
Copy link
Contributor

@flomonster flomonster left a comment

Choose a reason for hiding this comment

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

Great PR! Some changes will have to be made once the #3718 is merged.

editoast/src/infra.rs Outdated Show resolved Hide resolved
editoast/src/models/pathfinding.rs Show resolved Hide resolved
editoast/src/models/pathfinding.rs Show resolved Hide resolved
editoast/src/views/pathfinding/catenaries.rs Show resolved Hide resolved
editoast/src/views/pathfinding/catenaries.rs Outdated Show resolved Hide resolved
editoast/src/views/pathfinding/catenaries.rs Outdated Show resolved Hide resolved
editoast/src/views/pathfinding/catenaries.rs Show resolved Hide resolved
flomonster
flomonster requested changes Apr 11, 2023
View reviewed changes
editoast/src/views/pathfinding/catenaries.rs Outdated Show resolved Hide resolved
editoast/src/views/pathfinding/catenaries.rs Outdated Show resolved Hide resolved
leovalais
leovalais previously approved these changes Apr 11, 2023
View reviewed changes
Copy link
Contributor

@leovalais leovalais left a comment

Choose a reason for hiding this comment

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

LGTM, tested GET endpoint

@Castavo Castavo force-pushed the bpt/catenaries-on-path branch 3 times, most recently from fed1674 to 5ff7f1c Compare April 12, 2023 11:52
@Castavo Castavo requested a review from flomonster April 12, 2023 11:54
@leovalais leovalais mentioned this pull request Apr 12, 2023
Merged
flomonster
flomonster requested changes Apr 13, 2023
View reviewed changes
Copy link
Contributor

@flomonster flomonster left a comment

Choose a reason for hiding this comment

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

Almost good for me.

editoast/src/views/pathfinding/catenaries.rs Outdated Show resolved Hide resolved
editoast/src/error.rs Outdated Show resolved Hide resolved
@Castavo Castavo force-pushed the bpt/catenaries-on-path branch 2 times, most recently from 251fb7b to cedf0d1 Compare April 13, 2023 12:45
@Castavo Castavo force-pushed the bpt/catenaries-on-path branch 2 times, most recently from a146577 to a594101 Compare April 13, 2023 13:03
@Castavo Castavo requested a review from flomonster April 13, 2023 13:08
flomonster
flomonster approved these changes Apr 18, 2023
View reviewed changes
Copy link
Contributor

@flomonster flomonster left a comment

Choose a reason for hiding this comment

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

LGTM. I only have one suggestion left.

editoast/src/views/pathfinding/catenaries.rs Outdated Show resolved Hide resolved
flomonster
flomonster approved these changes Apr 18, 2023
View reviewed changes
editoast/src/views/pathfinding/catenaries.rs Outdated Show resolved Hide resolved
@Castavo Castavo force-pushed the bpt/catenaries-on-path branch 2 times, most recently from a2a3ae7 to 3fe9fc5 Compare April 18, 2023 12:24
@Castavo Castavo enabled auto-merge (rebase) April 18, 2023 12:24
@Castavo Castavo force-pushed the bpt/catenaries-on-path branch 2 times, most recently from 3ae6487 to a8f7fe7 Compare April 18, 2023 12:27
@Castavo Castavo merged commit e6612c5 into dev Apr 18, 2023
@Castavo Castavo deleted the bpt/catenaries-on-path branch April 18, 2023 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flomonster flomonster flomonster approved these changes

@leovalais leovalais leovalais left review comments

@Tguisnet Tguisnet Awaiting requested review from Tguisnet

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

Successfully merging this pull request may close these issues.

Accessing electrification on the path from the front
3 participants
@Castavo @flomonster @leovalais