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

feat: port autoware_interpolation from autoware.universe #149

Merged
merged 14 commits into from
Feb 12, 2025

Conversation

shulanbushangshu
Copy link
Contributor

@shulanbushangshu shulanbushangshu commented Jan 3, 2025

Description

Port autoware_interpolation from Autoware.Universe

Related links

#139
Parent Issue:

  • Link

How was this PR tested?

Notes for reviewers

Should be handled after these changes be merged:
#23

Interface changes

None.

Effects on system behavior

None.

Copy link

github-actions bot commented Jan 3, 2025

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

@shulanbushangshu shulanbushangshu changed the title feat: Port autoware_interpolation from Autoware.Universe feat: port autoware_interpolation from Autoware.Universe Jan 3, 2025
@shulanbushangshu shulanbushangshu changed the title feat: port autoware_interpolation from Autoware.Universe feat: port autoware_interpolation from autoware.universe Jan 3, 2025
@mitsudome-r mitsudome-r requested a review from rej55 January 7, 2025 15:31
Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

Since it’s a porting task, I’m keeping the review to a minimum.

common/autoware_interpolation/CHANGELOG.rst Outdated Show resolved Hide resolved
common/autoware_interpolation/package.xml Outdated Show resolved Hide resolved
common/autoware_interpolation/README.md Outdated Show resolved Hide resolved
Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

@youtalk
Copy link
Member

youtalk commented Jan 23, 2025

The error with pre-commit-optional resolved itself over time. Now we’re just waiting for the merge of #23.

Copy link

codecov bot commented Jan 24, 2025

Codecov Report

Attention: Patch coverage is 80.20699% with 153 lines in your changes missing coverage. Please review.

Project coverage is 79.91%. Comparing base (4cb18f5) to head (dca7701).
Report is 22 commits behind head on main.

Files with missing lines Patch % Lines
...n/test/src/test_spline_interpolation_points_2d.cpp 59.13% 1 Missing and 37 partials ⚠️
...nterpolation/test/src/test_interpolation_utils.cpp 52.63% 0 Missing and 27 partials ⚠️
...terpolation/src/spline_interpolation_points_2d.cpp 71.42% 23 Missing and 3 partials ⚠️
...terpolation/test/src/test_spline_interpolation.cpp 86.89% 0 Missing and 19 partials ⚠️
...n/test/src/test_spherical_linear_interpolation.cpp 72.41% 0 Missing and 16 partials ⚠️
...re_interpolation/test/src/test_zero_order_hold.cpp 86.48% 0 Missing and 10 partials ⚠️
...terpolation/test/src/test_linear_interpolation.cpp 83.72% 0 Missing and 7 partials ⚠️
...terpolation/src/spherical_linear_interpolation.cpp 76.47% 3 Missing and 1 partial ⚠️
...ude/autoware/interpolation/interpolation_utils.hpp 93.93% 0 Missing and 2 partials ⚠️
...utoware_interpolation/src/spline_interpolation.cpp 98.26% 1 Missing and 1 partial ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #149      +/-   ##
==========================================
+ Coverage   78.75%   79.91%   +1.16%     
==========================================
  Files          11       26      +15     
  Lines         193      966     +773     
  Branches       73      630     +557     
==========================================
+ Hits          152      772     +620     
- Misses         11       39      +28     
- Partials       30      155     +125     
Flag Coverage Δ *Carryforward flag
differential 80.20% <80.20%> (?)
total 78.75% <ø> (ø) Carriedforward from 40477de

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@NorahXiong
Copy link
Contributor

Hi, @youtalk, this PR is ready to be merged now.

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

Although it’s a porting PR, please just fix these small adjustments.

NorahXiong and others added 2 commits January 24, 2025 11:39
Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

LGTM

@liuXinGangChina
Copy link
Contributor

liuXinGangChina commented Jan 24, 2025

Hi mits san @mitsudome-r
This pr has been synced with main branch and receive “LGTM” from kondo san
Can you merge this one

Best regards

心刚

@youtalk
Copy link
Member

youtalk commented Jan 24, 2025

@liuXinGangChina It needs to be merged simultaneously with the PR that removes autoware_interpolation from autoware.universe.
https://github.com/autowarefoundation/autoware.universe/tree/main/common/autoware_interpolation

@NorahXiong
Copy link
Contributor

@liuXinGangChina It needs to be merged simultaneously with the PR that removes autoware_interpolation from autoware.universe. https://github.com/autowarefoundation/autoware.universe/tree/main/common/autoware_interpolation

Package autoware_interpolation is deleted from autoware.universe now.
autowarefoundation/autoware.universe#10018

Copy link
Member

@mitsudome-r mitsudome-r left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

LGTM

@mitsudome-r
Copy link
Member

Since the blocker PR was merged last week. I'm testing the build again.

@mitsudome-r
Copy link
Member

mitsudome-r commented Feb 10, 2025

I have tested build with this PR and the one in Autoware.Universe, but it seems like there are a lot of packages that depends on autoware_universe_utils through this package and got a lot of errors during build.

It's better to remove the dependency to autoware_universe_utils with autoware_utils in Autoware.Universe first to make sure that the build passes, and then do the porting. I am creating the following PRs to fix this issue:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants