set default value for arg Z of compose()#69
Open
jackie-wayland wants to merge 1 commit intomatthew-brett:mainfrom
Open
set default value for arg Z of compose()#69jackie-wayland wants to merge 1 commit intomatthew-brett:mainfrom
jackie-wayland wants to merge 1 commit intomatthew-brett:mainfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Change:
Introduce a default value for the
Zparameter in thecompose()function. WhenZis set toNone, it will be assumed to benp.ones(N). This modification will simplify the creation of common homogeneous transformation matrices, such ascompose(T, R), which are widely used in robotics.Justification:
Currently, the
compose()function requires explicit specification of theZparameter, even for simple transformations likecompose(T, R). This can be cumbersome and error-prone, as users may forget to includeZor pass an incorrect value.Setting a default value of
np.ones(N)forZwill address these issues by providing a convenient way to create homogeneous transformation matrices without the need for explicitZspecification. This will align with common usage patterns in robotics and other fields.Benefits:
Simplified function usage: Users can easily create common homogeneous transformation matrices without explicitly specifying
Z.Reduced error potential: The risk of forgetting or providing an incorrect
Zvalue is minimized.Improved code readability: Code using
compose()will be more concise and easier to understand.Impact:
This change will primarily affect users who rely on
compose()to create homogeneous transformation matrices. It will introduce a new default behavior for theZparameter, but this behavior aligns with common usage scenarios and simplifies the function's usage.Overall, the proposed change enhances the usability and convenience of the
compose()function, particularly for creating common homogeneous transformation matrices.