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

refactor: transformCoordinates methods without InternalSpacePoint class #1173

Merged
merged 30 commits into from
Mar 30, 2022

Conversation

CarloVarni
Copy link
Collaborator

Adding transformCoordinates functions that does not rely only on InternalSpacePoint. These methods will accept any space point format and require the user to define a function for retrieving coordinate and covariance values (i.e. std::function<std::array<float, 6>(const external_spacepoint_t&)>). This will allow users to use it externally to ACTS.

This does not remove the previous transformCoordinates functions in order to be backward compatible. However, they are adapted to use these new implementations.

@codecov
Copy link

codecov bot commented Feb 21, 2022

Codecov Report

Merging #1173 (f487d89) into main (5056fe5) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1173      +/-   ##
==========================================
- Coverage   47.86%   47.85%   -0.01%     
==========================================
  Files         371      371              
  Lines       19411    19413       +2     
  Branches     9145     9148       +3     
==========================================
  Hits         9291     9291              
- Misses       3789     3791       +2     
  Partials     6331     6331              
Impacted Files Coverage Δ
Core/include/Acts/Seeding/SeedFinderUtils.ipp 0.00% <0.00%> (ø)

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

CarloVarni and others added 2 commits February 21, 2022 12:03
Not Using std::function, Check callable + static_assert
@paulgessinger paulgessinger added this to the next milestone Feb 22, 2022
Carlo Varni and others added 2 commits March 1, 2022 18:02
Copy link
Contributor

@robertlangenberg robertlangenberg left a comment

Choose a reason for hiding this comment

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

This looks good, sorry for taking so long with the review!

@paulgessinger
Copy link
Member

@CarloVarni can you fix the CI?

@CarloVarni
Copy link
Collaborator Author

@paulgessinger I should have fixed the compilation issues now. I also found an inconsistency between the .ipp and .hpp files in a function declaration already present in main, which I solved (a const)

Copy link
Member

@paulgessinger paulgessinger left a comment

Choose a reason for hiding this comment

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

Let's go then.

@kodiakhq kodiakhq bot merged commit f7c32a8 into acts-project:main Mar 30, 2022
@paulgessinger paulgessinger modified the milestones: next, v18.0.0 Apr 4, 2022
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.

None yet

3 participants