Skip to content
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

Natural dtype for rotation matrices #81

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

asnt
Copy link

@asnt asnt commented Oct 14, 2018

Tentative PR for #80

This could maybe be applied to other functions. I haven't checked yet. Any hints welcome.

@asnt asnt force-pushed the natural-dtype branch 2 times, most recently from 25bd63c to 00476d4 Compare October 14, 2018 20:20
@adamlwgriffiths
Copy link
Owner

I have a feeling those test failures are related to #77

@adamlwgriffiths
Copy link
Owner

I think any of the create_from_x functions would need this.
It may also be pertinent to add to the quaternion functions.

Lastly, update the CHANGELOG and add yourself to the contributors in the README if you wish =)

There are other features we need to include in the CHANGELOG since the last release, but don't let that stop you.

@adamlwgriffiths
Copy link
Owner

Anyone else want to chime in on this one? @foohyfooh?

@asnt
Copy link
Author

asnt commented Nov 6, 2018

I think any of the create_from_x functions would need this.

The other create_from_X functions from matrix33.py seem to behave as expected. The ones taking euler angles or quaternions as input rely on the fact that the input dtype is float, which is most of the time the case but not guaranteed (see below).

It may also be pertinent to add to the quaternion functions.

As for the creation of a rotation matrix from axis-angle, I would also enforce that Euler angles and quaternions be float. Currently, the create function for both representation uses the input dtype, which might be int in some cases (although that will probably only happen rarely since the values are usually passed as float).
Problematic cases:

>>> import pyrr
>>> e = pyrr.euler.create(1,2,3)
>>> e
array([1, 2, 3])
>>> e.dtype
dtype('int64')
>>> q = pyrr.quaternion.create(1,2,3,4)
>>> q
array([1, 2, 3, 4])
>>> q.dtype
dtype('int64')
>>>

Seems related to #35

@adamlwgriffiths
Copy link
Owner

That's the thing. If you want to enable user laziness, then you need to update all the interfaces to be more intelligent. So many API functions need to be touched.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants