Skip to content

Conversation

@sea-bass
Copy link
Contributor

Created a small update to help solve this issue in Viser: nerfstudio-project/viser#483

Otherwise, the yourdfpy loader in this package only ever loads visual meshes/scenes, but not the collision ones.

I did default both parameters to true since it shouldn't hurt to always load everything by default, but let me know if this is not desired.

@coveralls
Copy link

coveralls commented Jun 18, 2025

Pull Request Test Coverage Report for Build 15748962655

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.979%

Totals Coverage Status
Change from base Build 15634305842: 0.0%
Covered Lines: 1115
Relevant Lines: 1138

💛 - Coveralls

@stephane-caron
Copy link
Member

stephane-caron commented Jun 18, 2025

Thank you for this proposal 👍

Regarding the defaults, I'd recommend we don't overriding upstream defaults (here the documentation of URDF.load states that load_collision_meshes defaults to False).

One thing we could do is add a **kwargs to the load_robot_description function itself, and let users forward to yourdfpy any of its load function parameters. This is for instance the approach we follow in another wrapper called qpsolvers (example). What do you think?

@sea-bass
Copy link
Contributor Author

Oh, the kwargs is a better idea! I'll do that and update the changelog.

@sea-bass sea-bass changed the title Add args to yourdfpy loader to optionally load visuals and collisions Allow passing kwargs to load_robot_description Jun 19, 2025
@sea-bass
Copy link
Contributor Author

@stephane-caron Updated!

@sea-bass sea-bass force-pushed the toggle-load-scenes branch from 56b46f7 to 17de398 Compare June 19, 2025 03:50
@sea-bass sea-bass changed the title Allow passing kwargs to load_robot_description Allow passing kwargs to yourdfpy.load_robot_description Jun 19, 2025
@stephane-caron stephane-caron merged commit 17de398 into robot-descriptions:main Jun 19, 2025
14 of 15 checks passed
@stephane-caron
Copy link
Member

Merged, thank you for taking care of this 👍

@sea-bass sea-bass deleted the toggle-load-scenes branch June 19, 2025 13:53
@sea-bass
Copy link
Contributor Author

Merged, thank you for taking care of this 👍

Thank you! Looking forward to the next released version to use this.

@stephane-caron
Copy link
Member

On it 😉

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants