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

Remove track links object #5203

Merged
merged 2 commits into from
Oct 12, 2023
Merged

Remove track links object #5203

merged 2 commits into from
Oct 12, 2023

Conversation

younesschrifi
Copy link
Contributor

Close #5171

@codecov
Copy link

codecov bot commented Oct 4, 2023 •
edited
Loading

Codecov Report

Merging #5203 (53d6ede) into dev (e32e129) will decrease coverage by 0.11%.
Report is 6 commits behind head on dev.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##                dev    #5203      +/-   ##
============================================
- Coverage     18.75%   18.65%   -0.11%     
+ Complexity     2318     2311       -7     
============================================
  Files           852      848       -4     
  Lines        103448   103362      -86     
  Branches       2408     2403       -5     
============================================
- Hits          19404    19282     -122     
- Misses        82706    82745      +39     
+ Partials       1338     1335       -3
Flag Coverage Δ
core 78.81% <ø> (-0.19%) ⬇️
editoast 70.08% <100.00%> (+0.06%) ⬆️
front 7.04% <ø> (-0.01%) ⬇️

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

Files Coverage Δ
...a/fr/sncf/osrd/railjson/schema/infra/RJSInfra.java 100.00% <ø> (ø)
...tion/tracks/undirected/UndirectedInfraBuilder.java 83.45% <ø> (-0.81%) ⬇️
editoast/src/converters/generate_routes.rs 99.23% <ø> (-0.04%) ⬇️
editoast/src/converters/osm_to_railjson.rs 87.50% <100.00%> (-0.56%) ⬇️
editoast/src/converters/utils.rs 88.07% <100.00%> (+0.70%) ⬆️
editoast/src/generated_data/error/mod.rs 85.00% <ø> (+0.38%) ⬆️
editoast/src/generated_data/error/routes.rs 100.00% <ø> (ø)
editoast/src/generated_data/mod.rs 96.72% <ø> (-0.16%) ⬇️
editoast/src/infra_cache/graph.rs 84.37% <100.00%> (-2.12%) ⬇️
editoast/src/infra_cache/mod.rs 76.58% <ø> (+1.24%) ⬆️
... and 16 more

... and 5 files with indirect coverage changes

@younesschrifi younesschrifi force-pushed the yci/remove-track-links branch 13 times, most recently from 93a660a to d7fdb6b Compare October 6, 2023 15:22
@Tguisnet Tguisnet force-pushed the yci/remove-track-links branch 11 times, most recently from 17581f4 to cf135ee Compare October 11, 2023 03:39
@Tguisnet Tguisnet marked this pull request as ready for review October 11, 2023 03:48
@Tguisnet Tguisnet requested review from a team and eckter as code owners October 11, 2023 03:48
@flomonster flomonster changed the title Yci/remove track links Remove track links object Oct 11, 2023
flomonster
flomonster reviewed Oct 11, 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!

  • Can you bump the railjson (at least minor) version
  • Can you rename your first commit message core: python: ...

editoast/migrations/2023-10-11-222214_delete_track_links/up.sql Show resolved Hide resolved
editoast/src/generated_data/error/switches.rs Outdated Show resolved Hide resolved
python/railjson_generator/railjson_generator/rjs_static/switch_types.json Show resolved Hide resolved
@younesschrifi younesschrifi force-pushed the yci/remove-track-links branch from cf135ee to 2295545 Compare October 11, 2023 13:55
@Castavo Castavo self-requested a review October 11, 2023 17:13
Castavo
Castavo reviewed Oct 11, 2023
View reviewed changes
Copy link
Contributor

@Castavo Castavo left a comment

Choose a reason for hiding this comment

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

You did not update python/railjson_generator/README.md, I don't know what the policy is about that
I think you should delete follow_track_link from python/railjson_generator/utils/generate_routes.py

You did not update editoast/map_layers.yaml

editoast/src/converters/utils.rs Outdated Show resolved Hide resolved
editoast/src/infra_cache/mod.rs Show resolved Hide resolved
editoast/src/infra_cache/graph.rs Show resolved Hide resolved
editoast/src/infra_cache/graph.rs Show resolved Hide resolved
editoast/src/schema/railjson.rs Outdated Show resolved Hide resolved
editoast/src/converters/osm_to_railjson.rs Outdated Show resolved Hide resolved
editoast/src/converters/utils.rs Show resolved Hide resolved
@Tguisnet Tguisnet force-pushed the yci/remove-track-links branch 2 times, most recently from 44edaea to 43bf12d Compare October 11, 2023 18:21
@Tguisnet
Copy link
Contributor

I think you should delete follow_track_link from python/railjson_generator/utils/generate_routes.py

Not sure about this, I think we need to adapt the function to the new switch link.

@younesschrifi younesschrifi force-pushed the yci/remove-track-links branch 2 times, most recently from d753a6a to 702a253 Compare October 11, 2023 21:35
@younesschrifi younesschrifi force-pushed the yci/remove-track-links branch from 702a253 to 53d6ede Compare October 12, 2023 08:17
Castavo
Castavo approved these changes Oct 12, 2023
View reviewed changes
Copy link
Contributor

@Castavo Castavo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Proof-read editoast and python
Tested simulation on an infra with the new switch type
Tested edition

clarani
clarani approved these changes Oct 12, 2023
View reviewed changes
Copy link
Contributor

@clarani clarani left a comment

Choose a reason for hiding this comment

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

LGTM and tested in local

flomonster
flomonster approved these changes Oct 12, 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.

Nice work!

@younesschrifi younesschrifi added this pull request to the merge queue Oct 12, 2023
Merged via the queue into dev with commit 54e7afa Oct 12, 2023
@younesschrifi younesschrifi deleted the yci/remove-track-links branch October 12, 2023 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tguisnet Tguisnet Tguisnet left review comments

@Castavo Castavo Castavo approved these changes

@clarani clarani clarani approved these changes

@flomonster flomonster flomonster approved these changes

@eckter eckter eckter approved these changes

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

Successfully merging this pull request may close these issues.

Delete the track link model
6 participants
@younesschrifi @Tguisnet @eckter @flomonster @clarani @Castavo