pyxem / orix

Analysing crystal orientations and symmetry in Python
https://orix.readthedocs.io
GNU General Public License v3.0
78 stars 45 forks source link

Add Sampling #460

Closed CSSFrancis closed 3 months ago

CSSFrancis commented 8 months ago

Description of the change

Add in Sampling using reduced symmetry sampling.

Sampling Methods Added:

The first sampling functionality mirrors the S_2 sampling tutorial, but is more explicit such that given some point group the rotations which rotate some vector [0,0,1] to each sampling vector is returned. This is used when fast template matching in pyxem.

The second sampling method is just a simple method for finding zone axes in simple systems. I think there are much better ways of doing this and I'd like to improve this method from the old diffsims one (and I think kikuchipy already does some of that).

Progress of the PR

Minimal example of the bug fix or new feature

from orix.symmetry import C2
from orix.sampling import get_sample_zone_axis
sampling = get_sample_zone_axis(2.0, point_group=C2)

For reviewers

CSSFrancis commented 8 months ago

@hakonanes Sorry for making you rerun the workflows a bunch....

CSSFrancis commented 8 months ago

@hakonanes Any now I didn't have pre-commit set up correctly.... We should (finally be good)

CSSFrancis commented 8 months ago

Hmm I'm not sure why my local pre commit isn't agreeing with orix. 😅 probably some version mismatch or it not running properly for some reason.

@hakonanes any problems with my adding the pre-commit ci to Orix?

That way it will run isort and black on the code either automatically or when you ask it to?

hakonanes commented 8 months ago

Hmm I'm not sure why my local pre commit isn't agreeing with orix

The two code style errors are related to sorting of imports. Do you have isort installed? It's among the dev dependencies.

@hakonanes any problems with my adding the pre-commit ci to Orix? That way it will run isort and black on the code either automatically or when you ask it to?

We could add a way to run pre-commit automatically if asked. I'm open to that. However, I think there is a point where there is too much automation in the development process for us to maintain, instead of focusing on functionality. I consider pushes by bots too much. At least for this project at this point in time.

Anyways, I wouldn't worry too much about the code style now. Could you explain a bit more in the top comment what this new functionality does and what its intended use is? Any details about the implementation you can provide would be much appreciated.

hakonanes commented 8 months ago

And, please request a review from me when you're ready!

hakonanes commented 8 months ago

Also, if you rebase on develop, the docs should be passing.

CSSFrancis commented 7 months ago

@hakonanes I've updated the head comment with more information.

CSSFrancis commented 3 months ago

pre-commit.ci autofix

pc494 commented 3 months ago

Should be able to review this for you tomorrow @CSSFrancis.

CSSFrancis commented 3 months ago

@pc494 Sounds good! I'll see if I can determine why the test isn't passing.

pc494 commented 3 months ago

@pc494 Sounds good! I'll see if I can determine why the test isn't passing.

I think that test might be the flaky one.

CSSFrancis commented 3 months ago

@pc494 yea I can't tell if that test is flakey or there is some scipy bug related to certain hardware configurations.