dfki-ric / pytransform3d

3D transformations for Python.
https://dfki-ric.github.io/pytransform3d/
Other
632 stars 68 forks source link

chore: draft change #203

Closed teunhoevenaars closed 1 year ago

teunhoevenaars commented 1 year ago

hi @AlexanderFabisch , I appreciate your work on this package! I have been playing around with it and am now building a small development tool that uses this package.

For my application it would be very helpful if fixed joints can also be identified based on their 'joint_name' and not only via their 'from_name' + 'to_name' combination. Please find attached a draft change.

Note:

Curious to hear your thoughts. Perhaps there is a clear reason not to do this. In that case I will solve it locally in my code.

AlexanderFabisch commented 1 year ago

Hi,

thanks for the contribution. Looks reasonable to me. The only concern that I have is that I don't know what get_joint_limits should return for this joint. Should it raise an error?

AlexanderFabisch commented 1 year ago

btw. @teunhoevenaars , maybe this fits your use case better: you can also directly call the function parse_urdf to get all information that is currently parsed from the URDF

teunhoevenaars commented 1 year ago

thanks for the suggestion. However, I really like the fact that the UrdfTransformManager contains all the information in one object. With this PR, I could just loop over _joints.items() (which is a protected attribute, but very useful for me) and perform work on them based on identification by name (instead of _toframe and _fromframe)

AlexanderFabisch commented 1 year ago

Although accessing protected attributes is not recommended, in this case I don't expect the interface to change in the near future, so I think that should be safe, too.

teunhoevenaars commented 1 year ago

let me know what you think of these latest changes

AlexanderFabisch commented 1 year ago

Apart from this comment I think it can be merged. I would just test the test coverage before that.

teunhoevenaars commented 1 year ago

Great to hear Alexander! I understand that testing the test coverage is something you will do, let me know if I've misunderstood or there is something I can do

AlexanderFabisch commented 1 year ago

Yes, exactly. Merged to develop branch. Thanks for your contribution!

teunhoevenaars commented 1 year ago

thanks @AlexanderFabisch! My aim is to remain involved, so hopefully we'll be in touch again soon enough. For my info; approximately when is the next Release scheduled / foreseen?

AlexanderFabisch commented 1 year ago

That sounds good. I am always happy to get some support!

There is no fixed release schedule. I usually update the version number after adding a few new features.