Skip to content

Added SignalStats pybind11 interface#343

Merged
chapulina merged 11 commits intoign-math6from
ahcorde/pybind11/signalstats
Dec 30, 2021
Merged

Added SignalStats pybind11 interface#343
chapulina merged 11 commits intoign-math6from
ahcorde/pybind11/signalstats

Conversation

@ahcorde
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde commented Dec 27, 2021

Signed-off-by: ahcorde ahcorde@gmail.com

🎉 New feature

Summary

This PR creates a SignalStats Pybind11 interface

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.

@ahcorde
Added SignalStats pybind11 interface
961cd0d 
Signed-off-by: Alejandro Hernández <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from scpeters as a code owner December 27, 2021 19:11
@ahcorde ahcorde self-assigned this Dec 27, 2021
@github-actions github-actions bot added Gazebo 1️1️ Dependency of Gazebo classic version 11 🌱 garden Ignition Garden 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress 🏰 citadel Ignition Citadel labels Dec 27, 2021
@ahcorde
Fixed tests
9c9996b 
Signed-off-by: Alejandro Hernández <ahcorde@gmail.com>
@ahcorde
make linters happy
eb0b9fc 
Signed-off-by: Alejandro Hernández <ahcorde@gmail.com>
@ahcorde
Merge branch 'ign-math6' into ahcorde/pybind11/signalstats
dc12c3e
@chapulina chapulina added the scripting Scripting interfaces to Ignition label Dec 27, 2021
chapulina
chapulina suggested changes Dec 27, 2021
View reviewed changes
Copy link
Copy Markdown
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.

There are linter warnings:

   /github/workspace/src/python_pybind11/src/SignalStats.cc:64:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
  /github/workspace/src/python_pybind11/src/SignalStats.cc:64:  Single-parameter constructors should be marked explicit.  [runtime/explicit] [5]
  /github/workspace/src/python_pybind11/src/SignalStats.cc:483:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
  Total errors found: 3

And test failures:

AttributeError: 'ignition.math.SignalMap' object has no attribute 'keys'

@ahcorde
make linters happy
8070b05 
Signed-off-by: Alejandro Hernández <ahcorde@gmail.com>
@ahcorde
Merge branch 'ahcorde/pybind11/signalstats' of https://github.com/ign…
7c942c8 
…itionrobotics/ign-math into ahcorde/pybind11/signalstats
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 27, 2021 •
edited
Loading

Codecov Report

Merging #343 (34f1e8a) into ign-math6 (f2b462d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           ign-math6     #343   +/-   ##
==========================================
  Coverage      99.65%   99.65%           
==========================================
  Files             67       67           
  Lines           6380     6380           
==========================================
  Hits            6358     6358           
  Misses            22       22

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 f2b462d...34f1e8a. Read the comment docs.

@ahcorde ahcorde mentioned this pull request Dec 29, 2021
Merged
8 tasks
@chapulina
Copy link
Copy Markdown
Contributor

chapulina commented Dec 29, 2021

This still has test failures:

   ERROR: test_02_signal_stats_intern_statistic (__main__.TestSignalStats)
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/github/workspace/src/python_pybind11/test/SignalStats_TEST.py", line 475, in test_02_signal_stats_intern_statistic
      self.assertEqual("max" in map.keys(), 1)
  AttributeError: 'ignition.math.SignalMap' object has no attribute 'keys'
  
  ----------------------------------------------------------------------

@ahcorde
Fixed test
c94b63a 
Signed-off-by: Alejandro Hernández <ahcorde@gmail.com>
@ahcorde
Merge branch 'ign-math6' into ahcorde/pybind11/signalstats
34f1e8a
@ahcorde ahcorde mentioned this pull request Dec 30, 2021
Merged
8 tasks
@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented Dec 30, 2021

@osrf-jenkins run tests please

@ahcorde ahcorde requested a review from chapulina December 30, 2021 13:48
chapulina
chapulina approved these changes Dec 30, 2021
View reviewed changes
Copy link
Copy Markdown
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.

Nice!

@chapulina chapulina merged commit 3fc159b into ign-math6 Dec 30, 2021
@chapulina chapulina deleted the ahcorde/pybind11/signalstats branch December 30, 2021 16:47
@osrf-triage
Copy link
Copy Markdown

osrf-triage commented Mar 1, 2022

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-03-01-citadel-edifice-fortress/1313/1

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

Reviewers

@scpeters scpeters Awaiting requested review from scpeters scpeters is a code owner

1 more reviewer

@chapulina chapulina chapulina approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ahcorde ahcorde

Labels

🏰 citadel Ignition Citadel 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress 🌱 garden Ignition Garden Gazebo 1️1️ Dependency of Gazebo classic version 11 scripting Scripting interfaces to Ignition

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ahcorde @chapulina @osrf-triage @j-rivero