-
Notifications
You must be signed in to change notification settings - Fork 3
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
Automatically detect sdf
extension from parsed string
#42
Automatically detect sdf
extension from parsed string
#42
Conversation
1b7fb0c
to
be460a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @flferretti for this contribution! This PR introduces the automatic detection of either the URDF or SDF type when the model description is a string containing the content. Before this PR, in this case the user had to pass a is_urdf
argument.
If the model is a pathlib.Path
, we read its extension for the file name, and it cannot be wrong. Having some heuristics on the URDF detection also when the passed description is a string, is definitely useful. However, the proposed solution is very fragile, and I suggested an more robust solution.
Furthermore, I'd not change the APIs by removing entirely the is_urdf
argument. With this PR, this argument will be useless in most cases, now extended to models passed as strings. Though, there might be edge cases where forcing either SDF or URDF is still necessary, and I'd suggest to leave this is_urdf
argument there to address them. We can update the docstring to something like: Force the SDF resource to be treated as URDF if the automatic detection fails.
be460a8
to
e6cf506
Compare
e6cf506
to
804b95c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @flferretti for your contribution.
This PR enable detecting the XML type from the parsed string.