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

Added Particle Emitter Python interface #999

Merged
merged 9 commits into from
Jun 29, 2022
Merged

Conversation

ahcorde
Copy link
Collaborator

@ahcorde ahcorde commented Apr 21, 2022

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

🎉 New feature

This PR is part of this meta ticket #931

Summary

Added Particle Emitter Python interface

Test it

from sdformat import ParticleEmitter

particleEmitter = ParticleEmitter()

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.

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
ahcorde Alejandro Hernández Cordero
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde added scripting Scripting interfaces to Ignition mujoco labels Apr 21, 2022
@ahcorde ahcorde requested review from azeey and scpeters as code owners April 21, 2022 16:39
@ahcorde ahcorde self-assigned this Apr 21, 2022
@ahcorde ahcorde mentioned this pull request Apr 21, 2022
62 tasks
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Apr 21, 2022
ahcorde added 2 commits May 4, 2022 13:41

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@codecov
Copy link

codecov bot commented May 12, 2022

Codecov Report

Merging #999 (97d5607) into main (a286828) will decrease coverage by 0.15%.
The diff coverage is 55.76%.

@@            Coverage Diff             @@
##             main     #999      +/-   ##
==========================================
- Coverage   83.39%   83.23%   -0.16%     
==========================================
  Files         151      152       +1     
  Lines       18490    18594     +104     
==========================================
+ Hits        15419    15477      +58     
- Misses       3071     3117      +46     
Impacted Files Coverage Δ
python/src/sdf/pyParticleEmitter.cc 55.33% <55.33%> (ø)
python/src/sdf/_ignition_sdformat_pybind11.cc 100.00% <100.00%> (ø)

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 a286828...97d5607. Read the comment docs.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@chapulina chapulina changed the title Added Particle Emitter interface Added Particle Emitter Python interface Jun 2, 2022
Copy link
Collaborator

@azeey azeey left a comment

Choose a reason for hiding this comment

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

Looks great. Just a few minor comments

Comment on lines 23 to 26
#include "sdf/Box.hh"
#include "sdf/Capsule.hh"
#include "sdf/Cylinder.hh"
#include "sdf/Ellipsoid.hh"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need all the other headers

Suggested change
#include "sdf/Box.hh"
#include "sdf/Capsule.hh"
#include "sdf/Cylinder.hh"
#include "sdf/Ellipsoid.hh"
#include "sdf/ParticleEmitter.hh"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: The headers below ParticleEmitter can be removed as well.

@ahcorde ahcorde requested a review from azeey June 29, 2022 00:16

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link
Collaborator

@azeey azeey left a comment

Choose a reason for hiding this comment

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

One last minor comment, otherwise LGTM!

Comment on lines 23 to 26
#include "sdf/Box.hh"
#include "sdf/Capsule.hh"
#include "sdf/Cylinder.hh"
#include "sdf/Ellipsoid.hh"
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: The headers below ParticleEmitter can be removed as well.

ahcorde added 2 commits June 29, 2022 09:49
Signed-off-by: ahcorde <ahcorde@gmail.com>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@ahcorde ahcorde merged commit 50032e9 into main Jun 29, 2022
@ahcorde ahcorde deleted the ahcorde/python/particleemitter branch June 29, 2022 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden mujoco scripting Scripting interfaces to Ignition
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants