Skip to content

Adds python interface to Filter, MovingWindowFilter, RotationSpline.#230

Merged
scpeters merged 16 commits intoign-math6from
LolaSegura/add_filter_moving-window-filter_rotation-spline
Sep 18, 2021
Merged

Adds python interface to Filter, MovingWindowFilter, RotationSpline.#230
scpeters merged 16 commits intoign-math6from
LolaSegura/add_filter_moving-window-filter_rotation-spline

Conversation

@LolaSegura
Copy link
Copy Markdown
Contributor

@LolaSegura LolaSegura commented Aug 30, 2021 •
edited
Loading

🎉 New feature

Goes on top of #221
Related to #101 #210

Summary

Adds Python interface for three math classes: Filter, MovingWindowFilter and RotationSPline. For each class a python test has been created.

Related issues and notes

Filter

The Filter.hh file contains various template classes that inherit from a Filter class, which is a template class itself. To create binding for this classes an instance of Filter<type> had to be declared previously.
There were some classes that inherit from an instance of another class. For example class OnePoleQuaternion : public OnePole<math::Quaterniond>. This wasn't being interpreted by swig as expected. The inherited methods that were not override, were not being created in the binding. The solution we implemented for this was define these classes inside the interface file without the inheritance and adding the methods that were not being created. A similar approach was taken in the SignalStats class (#220 ).

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

@LolaSegura
Adds scripting interface to Quaternion and a python test
a22cf39 
Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>
@LolaSegura
Adds scripting interface to Matrix3 and a python test
aec4a6f 
Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>
@LolaSegura
Adds scripting interface to Pose3 and a python test
44228a1 
Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>
@LolaSegura
Solves bugg in the Reset() method inside Pose3
b8aca58 
Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>
@LolaSegura
Adds scripting interface to Matrix4 and a python test
6da3282 
Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>
@LolaSegura
Solves bugg in the Construct test for Matrix4 class
9d8f5f9 
Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>
@LolaSegura
Adds %rename tag to interface files in order to match pep-8 naiming s…
7ac05b6 
…tyle.

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>
@LolaSegura
Updates the test to follow the naiming style and moves the .i and pyt…
88dc6f6 
…hon test to the python folder.

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>
@LolaSegura
Address comments.
f76a40a 
Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>
@LolaSegura
Adds a python method to convert from a Matrix3 to a Quaternion.
e46c44b 
Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>
@LolaSegura
Adds to_quaternion() method to Matrix3.
d885997 
Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>
@LolaSegura
Adds python interface to Filter, MovingWindowFilter, RotationSpline.
d26e61e 
Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>
@github-actions github-actions bot added Gazebo 1️1️ Dependency of Gazebo classic version 11 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome labels Aug 30, 2021
@LolaSegura
Deletes extra blank spaces.
fac1c96 
Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>
@francocipollone
Copy link
Copy Markdown
Contributor

francocipollone commented Aug 30, 2021

This PR is on hold until #221 is merged.

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 30, 2021 •
edited
Loading

Codecov Report

Merging #230 (2eb2bab) into ign-math6 (b54dce5) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           ign-math6     #230   +/-   ##
==========================================
  Coverage      99.40%   99.40%           
==========================================
  Files             66       66           
  Lines           6185     6185           
==========================================
  Hits            6148     6148           
  Misses            37       37

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 b54dce5...2eb2bab. Read the comment docs.

@LolaSegura LolaSegura mentioned this pull request Aug 30, 2021
Closed
45 tasks
francocipollone
francocipollone approved these changes Aug 30, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@francocipollone francocipollone left a comment

Choose a reason for hiding this comment

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

LGTM

@francocipollone francocipollone marked this pull request as ready for review August 30, 2021 15:50
@chapulina chapulina added the beta Targeting beta release of upcoming collection label Sep 8, 2021
@francocipollone
Copy link
Copy Markdown
Contributor

francocipollone commented Sep 8, 2021

I'll solve the conflicts after #221 is merged.

azeey
azeey reviewed Sep 17, 2021
View reviewed changes
src/python/MovingWindowFilter.i Outdated
#include "ignition/math/Vector3.hh"
%}

%import "std_vector.i"
Copy link
Copy Markdown
Contributor

@azeey azeey Sep 17, 2021

Choose a reason for hiding this comment

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

Removing this line fixes the compilation error for me. Maybe it's a known issue, but I had to touch python.i for it to recompile this file.

Copy link
Copy Markdown
Contributor

@francocipollone francocipollone Sep 17, 2021

Choose a reason for hiding this comment

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

Let me take a look. It seemed to be working before merging this branch with ign-math6.

Copy link
Copy Markdown
Contributor

@francocipollone francocipollone Sep 17, 2021

Choose a reason for hiding this comment

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

You were right. That import was causing issues because of the syntaxis: it should be include instead of import

The truth is that that import wasn't necessary given that std::vector isn't being used in the API, so that import statement wasn't needed at all. I pushed the fix

@francocipollone
Removes unused includes.
7ebbcc7 
Signed-off-by: Franco Cipollone <franco.c@ekumenlabs.com>
@francocipollone francocipollone force-pushed the LolaSegura/add_filter_moving-window-filter_rotation-spline branch from 6d6708d to 7ebbcc7 Compare September 17, 2021 16:15
@scpeters
fix whitespace
2eb2bab 
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters scpeters merged commit e311b3b into ign-math6 Sep 18, 2021
@scpeters scpeters deleted the LolaSegura/add_filter_moving-window-filter_rotation-spline branch September 18, 2021 05:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@azeey azeey azeey left review comments

@scpeters scpeters scpeters approved these changes

+1 more reviewer

@francocipollone francocipollone francocipollone approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

beta Targeting beta release of upcoming collection 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress Gazebo 1️1️ Dependency of Gazebo classic version 11

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@LolaSegura @francocipollone @azeey @scpeters @j-rivero @chapulina @osrf-triage