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

Add Vector3d.scatter() reproject onto hemisphere functionality #307

Merged
merged 13 commits into from
Mar 29, 2022

Conversation

harripj
Copy link
Collaborator

@harripj harripj commented Mar 27, 2022

Description of the change

As discussed with @hakonanes in #306 (comment)

This is just an idea for the future, but it might be that we should support a boolean parameter to Vector3d.scatter() to set whether vectors on the other hemisphere (northern if hemisphere="south is passed and vice versa) should be projected up to the projection plane as well. We should then force the scatter points to be different.

We should include the option to reproject vectors onto the visible hemisphere, ie. if Vector3d.scatter(hemisphere='upper') is called then vectors projected onto the lower hemisphere are not shown. This PR adds the reproject argument to Vector3d.scatter() which reprojects vectors located on the hidden hemisphere to the visible hemisphere. This is achieved by reflection of the vectors in the x-y plane.

The reprojected vectors are displayed with a '+' marker by default and can be set using the reproject_marker argument. If hemisphere='both', reproject=True has no effect as all vectors are already shown on one of the two axes.

If approved the changes in this PR should be used to simplify #306.

Progress of the PR

Minimal example of the bug fix or new feature

from orix.quaternion import Orientation, symmetry
from orix.vector import Vector3d

o = symmetry.O * Orientation.random()
v = o * Vector3d.zvector()

v.scatter(hemisphere='upper', show_hemisphere_label=True)

image

v.scatter(hemisphere='upper', reproject=True, show_hemisphere_label=True)

image

reproject=True has no effect if hemisphere='both':

v.scatter(hemisphere='both', reproject=True, show_hemisphere_label=True)

image

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.

@harripj harripj added the enhancement New feature or request label Mar 27, 2022
@hakonanes hakonanes added this to the v0.9.0 milestone Mar 28, 2022
@hakonanes
Copy link
Member

hakonanes commented Mar 28, 2022

Just ping me whenever this is ready for review.

EDIT: I typically use the "Mark as draft" and "Mark as ready for review" functionality.

@harripj harripj requested a review from hakonanes March 28, 2022 08:20
Copy link
Member

@hakonanes hakonanes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very nice addition to the scatter() convenience plotting method! I've made some requests. I think a mention of this possibility in the stereographic projection user guide should be added. Perhaps it is most suitable to add it to the end of the "Upper and/or lower hemisphere" part (here)?

orix/vector/vector3d.py Show resolved Hide resolved
orix/vector/vector3d.py Outdated Show resolved Hide resolved
orix/vector/vector3d.py Outdated Show resolved Hide resolved
orix/plot/stereographic_plot.py Show resolved Hide resolved
@harripj harripj requested a review from hakonanes March 28, 2022 14:14
Copy link
Member

@hakonanes hakonanes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the docstring improvements and addition to the user guide. I've got a few more requests before approving.

orix/vector/vector3d.py Outdated Show resolved Hide resolved
orix/vector/vector3d.py Outdated Show resolved Hide resolved
orix/vector/vector3d.py Show resolved Hide resolved
orix/tests/test_vector3d.py Show resolved Hide resolved
@harripj harripj mentioned this pull request Mar 28, 2022
15 tasks
@hakonanes hakonanes merged commit 0d97520 into pyxem:master Mar 29, 2022
@harripj harripj deleted the vector_scatter_reproject_hemisphere branch March 29, 2022 09:07
harripj added a commit to harripj/orix that referenced this pull request Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants