search

facebookresearch / tacto

Simulator of vision-based tactile sensors.
MIT License
333 stars 75 forks source link

Add pybullet client id #6

Closed mees closed 3 years ago

mees commented 3 years ago

PR for issue #1

facebook-github-bot commented 3 years ago

Hi @mees!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

facebook-github-bot commented 3 years ago

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

poweic commented 3 years ago

Hi @mees, Thanks for the PR!

Adding self.cid means that the users have to know the client id before instantiating a Sensor object. They also need to be very careful not to pass the object/link id of the wrong client when calling add_camera and add_object. But I think we can revisit this if this become an issue.

Can you use black (specified in .pre-commit-config.yaml) to format the code in tacto/sensor.py? This can be done either by remaking the commit with pre-commit installed or running pre-commit manually (i.e., pre-commit run --files tacto/sensor.py && git commit).

Other than that, this looks good to me.

mees commented 3 years ago

Thanks @poweic, I totally forgot about pre-commit, let me know if this resolves the issue.

mees commented 3 years ago

BTW, shouldn't pre-commit be then listed in the requierements.txt file ?

poweic commented 3 years ago

Thanks @mees . It's now merged.

Yes, pre-commit should be listed in requirements/dev.txt. I will push a fix later.