Replace numpy-quaternion with quaternionic.

CSSFrancis commented 4 months ago

Description of the change

As per #506.

I'm running into some problems making this change becuase I'm not really sure what the improper setter functions works with the _data attribute.

The difference arrises from quaternionic forcing having 4 as the last shape and not automatically casting as the numpy-quaternion package does.


import quaternionic as quaternion2
import quaternion
import numpy as np

quaternion.as_float_array(np.ones((10,4))).shape 
# (10, 4, 4)

quaternion2.array(np.ones((10,4))).shape
# (10,4)

@hakonanes any advice for what might need to change as a result?

Progress of the PR

[ ] Docstrings for all functions
[ ] Unit tests with pytest for all lines
[ ] Clean code style by running black via pre-commit

For reviewers

[ ] The PR title is short, concise, and will make sense 1 year later.
[ ] New functions are imported in corresponding __init__.py.
[ ] New features, API changes, and deprecations are mentioned in the unreleased section in CHANGELOG.rst.
[ ] Contributor(s) are listed correctly in __credits__ in orix/__init__.py and in .zenodo.json.

CSSFrancis commented 4 months ago

Okay so we might have to adjust how we do things.

I think that the data for a Quaternion should always be a quaternion class from quaternionic.
The rotation class should be Quaternion + a boolean for proper/ improper. I think the right thing to do would be to make a numpy array class for this but.... Maybe we should just sperate these two arrays into data + improper.

pc494 commented 3 months ago

It looks like we might get a downstream fix now, which would be excellent.

https://github.com/moble/quaternion/issues/229

CSSFrancis commented 3 months ago

@pc494 thank goodness:) that was quickly turning into a bit of a nightmare to sort out

hakonanes commented 2 months ago

Sorry for answering late here...

The dependency on numpy-quaternion is NOT important. We only use it for quaternion-quaternion and quaternion-vector multiplication. Our "old" implementation used Numba, and as far as I can remember, it was only twice as slow.

If numpy-quaternion becomes a hassle, I'd suggest we make it an optional dependency: use it when available, but fall back to Numba if it's not. What do you think?

hakonanes commented 2 months ago

And I should say, I find storing the rotation handedness (proper -> right-handed, improper -> left-handed; or, if you will, the rotation followed by an inversion) as a fifth value 0 or 1 in the Rotation._data array is rather elegant and makes the implementation easier.

pc494 commented 2 months ago

I think this is safe to close.

@CSSFrancis any objection?

pyxem / orix