Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add Interval class #388

Merged
merged 1 commit into from
Mar 18, 2022
Merged

Add Interval class #388

merged 1 commit into from
Mar 18, 2022

Conversation

hidmic
Copy link
Contributor

@hidmic hidmic commented Mar 16, 2022

🎉 New feature

Summary

Split from #387. This PR introduces an Interval class to represent scalar, real intervals.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic hidmic requested a review from scpeters as a code owner March 16, 2022 18:51
@hidmic
Copy link
Contributor Author

hidmic commented Mar 16, 2022

@chapulina PTAL!

Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Do you mind to add this a Python interfaces ? or if you don't have time please open an issue.

@github-actions github-actions bot added the 🌱 garden Ignition Garden label Mar 16, 2022
@codecov
Copy link

codecov bot commented Mar 16, 2022

Codecov Report

Merging #388 (2adb728) into main (0d05f47) will increase coverage by 0.08%.
The diff coverage is 100.00%.

❗ Current head 2adb728 differs from pull request most recent head b6fbc87. Consider uploading reports for the commit b6fbc87 to get more accurate results

@@            Coverage Diff             @@
##             main     #388      +/-   ##
==========================================
+ Coverage   99.64%   99.72%   +0.08%     
==========================================
  Files          65       66       +1     
  Lines        6115     6189      +74     
==========================================
+ Hits         6093     6172      +79     
+ Misses         22       17       -5     
Impacted Files Coverage Δ
include/ignition/math/Interval.hh 100.00% <100.00%> (ø)
include/ignition/math/Plane.hh 100.00% <0.00%> (ø)
include/ignition/math/Matrix4.hh 100.00% <0.00%> (ø)
include/ignition/math/Inertial.hh 100.00% <0.00%> (ø)
include/ignition/math/MassMatrix3.hh 99.39% <0.00%> (+<0.01%) ⬆️
include/ignition/math/Quaternion.hh 100.00% <0.00%> (+0.24%) ⬆️
include/ignition/math/Helpers.hh 100.00% <0.00%> (+0.69%) ⬆️
src/Helpers.cc 98.21% <0.00%> (+2.67%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebd1a13...b6fbc87. Read the comment docs.

@chapulina chapulina added the MBARI-LRAUV Sponsored by MBARI-LRAUV project: https://github.com/osrf/lrauv label Mar 16, 2022
@chapulina chapulina self-requested a review March 16, 2022 22:03
@hidmic
Copy link
Contributor Author

hidmic commented Mar 17, 2022

Do you mind to add this a Python interfaces ? or if you don't have time please open an issue.

@ahcorde I'll open an issue once we merge this PR. I promise I'll get back and implement pybind11 bindings later on, but I need this feature in ASAP.

Copy link
Contributor

@chapulina chapulina 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 great, thanks for the tests, documentation and example. I just have minor comments, but nothing blocking.

@chapulina chapulina dismissed ahcorde’s stale review March 18, 2022 18:13

Python interfaces will be added on follow up PRs.

@chapulina chapulina merged commit 5f3c3ed into gazebosim:main Mar 18, 2022
@hidmic hidmic deleted the add-interval-class branch March 18, 2022 18:14
@hidmic hidmic mentioned this pull request Mar 18, 2022
7 tasks
@hidmic hidmic mentioned this pull request Mar 23, 2022
39 tasks

namespace detail {
template<typename T>
const Interval<T> gUnboundedInterval =
Copy link
Member

Choose a reason for hiding this comment

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

there may be some issues with static initialization of this variable (see #269). It looks like this is almost following the pattern from #283, but it is not a constexpr, so I think some additional work will be needed

ahcorde pushed a commit that referenced this pull request Jul 4, 2022
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
chapulina pushed a commit that referenced this pull request Jul 8, 2022
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🌱 garden Ignition Garden MBARI-LRAUV Sponsored by MBARI-LRAUV project: https://github.com/osrf/lrauv
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants