Adds python interface to RollingMean, Color and Spline by LolaSegura · Pull Request #218 · gazebosim/gz-math

LolaSegura · 2021-08-10T16:26:03Z

🎉 New feature

Goes on top of #216.

Related to #101 #210

Summary

Adds Python interface for three math classes: RollingMean, Color, Spline. For each class a python test has been created.

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

codecov · 2021-08-10T16:40:09Z

Codecov Report

Merging #218 (9e6df0f) into ign-math6 (b3804bf) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           ign-math6     #218   +/-   ##
==========================================
  Coverage      99.22%   99.22%           
==========================================
  Files             66       66           
  Lines           6162     6162           
==========================================
  Hits            6114     6114           
  Misses            48       48

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 b3804bf...9e6df0f. Read the comment docs.

LolaSegura · 2021-08-12T15:25:50Z

General comments

Given that there is no assignment in python the operator=() is ignored by swig. So the only way left to copy elements of the custom classes is by using the copy constructor.

The operator[]() was being ignored as well and this warning was shown when trying to build the ignition-math6 module Warning 389: operator[] ignored (consider using %extend). So as suggested to implement the indexation method we decided to create an extension.

The operator<<() had to be implemented using an %extend modifier as well. Also, for it to work correctly the std_string.i file had to be included.

Color

In C++ the getter methods are overloaded, so for example, getting the r element of a color could be done with both of this methods:
float R() const;
float &R();
But when creating the python interface swig interpreted this as if one method was shadowing the other. Given that a setter method is defined as well (void R(const float _r)) the decision was to implement only the first one.

francocipollone · 2021-08-16T13:53:04Z

@LolaSegura Please create another PR with the files related to Matrix3, Matrix4, Pose3, and Quaternion in order
to avoid this PR to grow this much and make it easier to review.

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

…tyle. Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

…hon test to the python folder. Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

francocipollone

Nice! Just a few comments

src/python/Color.i

src/python/Color_TEST.py

src/python/RollingMean.i

francocipollone · 2021-08-20T20:51:02Z

src/python/RollingMean.i

+      #endif
+    };
+  }
+}


nit: Add an extra empty line at the end

src/python/Spline.i

src/python/Spline_TEST.py

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

francocipollone

LGTM!

francocipollone · 2021-08-23T16:09:18Z

@scpeters @chapulina It is ready for an extra review!

scpeters · 2021-08-30T20:44:56Z

I just resolved conflicts after merging #220

src/python/CMakeLists.txt

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

…tionrobotics/ign-math into LolaSegura/python_interface

LolaSegura requested a review from francocipollone August 10, 2021 16:26

github-actions bot added 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome labels Aug 10, 2021

francocipollone changed the title ~~Lola segura/python interface~~ Adds python interfaco to RollingMean, Color and Spline Aug 10, 2021

francocipollone changed the title ~~Adds python interfaco to RollingMean, Color and Spline~~ Adds python interface to RollingMean, Color and Spline Aug 10, 2021

LolaSegura force-pushed the LolaSegura/python_interface branch from aaaa60a to 895eb1e Compare August 11, 2021 15:34

LolaSegura marked this pull request as ready for review August 12, 2021 18:59

LolaSegura requested a review from scpeters as a code owner August 12, 2021 18:59

LolaSegura force-pushed the LolaSegura/python_interface branch 2 times, most recently from 71a152d to b03e678 Compare August 18, 2021 18:15

francocipollone mentioned this pull request Aug 19, 2021

Python interface: Renames methods to match PEP8 style #226

Merged

8 tasks

LolaSegura added 6 commits August 20, 2021 10:51

Resolve conflicts with ign-math6.

ce7b347

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

Adds scripting interface to Color and a python test.

9d91a25

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

Adds scripting interface to Spline and a python test.

abaf238

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

Adds python support for operator[] and operator<< to the Color class.

d96a2a2

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

Adds %rename tag to interface files in order to match pep-8 naiming s…

7dfb6ec

…tyle. Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

Updates the test to follow the naiming style and moves the .i and pyt…

c0407ad

…hon test to the python folder. Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

LolaSegura force-pushed the LolaSegura/python_interface branch from b03e678 to c0407ad Compare August 20, 2021 14:31

LolaSegura added 2 commits August 20, 2021 12:10

Adds swig files to the python/CMakeList.

73ca310

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

Tracks moved files.

97af549

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

francocipollone suggested changes Aug 20, 2021

View reviewed changes

Address comments.

01a0c47

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

LolaSegura requested a review from francocipollone August 23, 2021 13:07

Alphasort Spline.

ea69351

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

francocipollone approved these changes Aug 23, 2021

View reviewed changes

francocipollone requested a review from chapulina August 24, 2021 23:03

francocipollone mentioned this pull request Aug 26, 2021

Documentation, Style and scripting interfaces update #101

Closed

45 tasks

scpeters added 2 commits August 30, 2021 11:58

Merge branch 'ign-math6' into LolaSegura/python_interface

6f5b765

Merge branch 'ign-math6' into LolaSegura/python_interface

f4db413

scpeters reviewed Aug 30, 2021

View reviewed changes

src/python/CMakeLists.txt Outdated Show resolved Hide resolved

LolaSegura and others added 4 commits August 31, 2021 10:28

Deletes extra swig_files inside CMakeList file.

1cd3bdf

Signed-off-by: LolaSegura <lsegura@ekumenlabs.com>

Merge branch 'LolaSegura/python_interface' of https://github.com/igni…

27464b8

…tionrobotics/ign-math into LolaSegura/python_interface

Merge branch 'ign-math6' into LolaSegura/python_interface

94f7c51

Merge branch 'ign-math6' into LolaSegura/python_interface

9a40ad9

chapulina added the beta Targeting beta release of upcoming collection label Sep 8, 2021

Merge branch 'ign-math6' into LolaSegura/python_interface

9e6df0f

scpeters approved these changes Sep 8, 2021

View reviewed changes

scpeters merged commit c3ab4dc into ign-math6 Sep 8, 2021

scpeters deleted the LolaSegura/python_interface branch September 8, 2021 17:43

+                    #endif
+                  };
+                }
+              }

                
                    No newline at end of file

Conversation

LolaSegura commented Aug 10, 2021 • edited by francocipollone Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎉 New feature

Summary

Checklist

Uh oh!

codecov bot commented Aug 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

LolaSegura commented Aug 12, 2021

General comments

Color

Uh oh!

francocipollone commented Aug 16, 2021

Uh oh!

francocipollone left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

francocipollone Aug 20, 2021

Choose a reason for hiding this comment

Uh oh!

LolaSegura Aug 23, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

francocipollone left a comment

Choose a reason for hiding this comment

Uh oh!

francocipollone commented Aug 23, 2021

Uh oh!

scpeters commented Aug 30, 2021

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

LolaSegura commented Aug 10, 2021 •

edited by francocipollone

Loading

codecov bot commented Aug 10, 2021 •

edited

Loading