Sanitize direction2color inputs by argerlt · Pull Request #639 · pyxem/orix

argerlt · 2026-03-05T01:17:11Z

Description of the change

This should solve #638:

The input 'direction' in IPFColorKeyTSL must now be a sample vector (Miller is not allowed)
direction2color now accepts both crystal and sample vectors, as long as the crystals are of the same Laue class as the color key.

Progress of the PR

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

Minimal example of the bug fix or new feature

import numpy as np

import orix.crystal_map as ocm
import orix.plot as opl
import orix.quaternion as oqu
import orix.quaternion.symmetry as osm
import orix.vector as ove

p1 = ocm.Phase(name="fakename", point_group=osm.Oh)
p2 = ocm.Phase(name="fakename", point_group=osm.D6h)
arr = np.linspace(0, 5, 30).reshape(10, 3)
v = ove.Vector3d(arr)
g1 = oqu.Orientation.random(5, symmetry=p1.point_group)
g2 = oqu.Orientation.random(5, symmetry=p2.point_group)
m1 = ove.Miller(xyz=arr, phase=p1)
m2 = ove.Miller(xyz=arr, phase=p2)

These operations are allowed:

ckey_dir = opl.DirectionColorKeyTSL(osm.Oh)
rbg = ckey_dir.direction2color(v)
rbg = ckey_dir.direction2color(m1)

ckey_ori = opl.IPFColorKeyTSL(symmetry=osm.Oh, direction=v[0])
rbg = ckey_ori.orientation2color(g1)

While these are not, and also throw informative errors

# ckey_ori = opl.IPFColorKeyTSL(symmetry=osm.Oh, direction=m1)
>>> ValueError: The Viewing direction must a real space vector (Vector3d), not a cystal vector (Miller)

# ckey_ori = opl.IPFColorKeyTSL(symmetry=osm.Oh, direction=[1,1,1,1])
>>> ValueError: 'direction' cannot be interpreted as a Vector3d

# ckey_ori = opl.IPFColorKeyTSL(symmetry=osm.Oh, direction=v)
>>> ValueError: 'direction' only accepts a single vector as an input

# rbg = ckey_dir.direction2color(m2)
>>> ValueError: 'direction' has a Laue group of 6/mmm, which differs  from the Laue group m-3m used by this color key

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.

argerlt · 2026-03-05T01:17:55Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

hakonanes · 2026-03-05T20:59:13Z

Thank you for making this fix. I don't have time right now, but I'll go through it soon when I have.

Just quickly, passing a vector to orientation2color() doesn't make any sense to me. It shouldn't be supported.

argerlt · 2026-03-05T21:03:21Z

Passing a vector to orientation2color()

Ah, I agree. Code itself is correct, but my description in this PR is wrong, as is the changelog entry. I'll fix it.

…/orix into sanitize_direction2color

argerlt · 2026-03-06T00:06:09Z

pre-commit.ci autofix

for more information, see https://pre-commit.ci

hakonanes

Good changes! I've got a couple of suggestions and requests for improvements.

hakonanes · 2026-03-08T07:27:44Z

orix/plot/direction_color_keys/direction_color_key_tsl.py

+            h_laue = direction.phase.point_group.laue.name
+            if h_laue == self.symmetry.name:


I suggest to use the Laue class equality here instead of just the name. The equality check uses the symmetry hash, which includes both the name and rotations (quaternion and handedness).

direction.phase.point_group.laue == self.symmetry

hakonanes · 2026-03-08T07:35:39Z

orix/plot/orientation_color_keys/ipf_color_key_tsl.py

        direction : orix.vector.Vector3d, optional
-            Sample direction. If not given, sample Z direction (out of
-            plane) is used.
+            Viewing direction the defines the IPF coloring. for the TSL


Do we need to mention fibers here? The sample direction gives the crystal direction which colors the orientation. I think this is a simpler explanation. And the old "If not given, ..." description should be good still.

hakonanes · 2026-03-08T07:37:07Z

orix/plot/orientation_color_keys/ipf_color_key_tsl.py

-            Sample direction. If not given, sample Z direction (out of
-            plane) is used.
+            Viewing direction the defines the IPF coloring. for the TSL
+            colormap, this defines the fiber of orientations that will be


Could you:

Move the parameter description to the class docstring

Remove this docstring

Otherwise it won't show in our API reference.

This is a remnant of my limited documentation skills some years ago...

hakonanes · 2026-03-08T07:39:57Z

orix/plot/orientation_color_keys/ipf_color_key_tsl.py

I'm seeing now that the orientation color key inheritance is poorly implemented. If it's fine with you, I'd like to make a few updates to your branch?

hakonanes · 2026-03-08T07:41:18Z

orix/plot/orientation_color_keys/ipf_color_key_tsl.py

+        if direction is not None:
+            if isinstance(direction, Miller):
+                raise ValueError(
+                    "The Viewing direction must a real space vector (Vector3d), not a cystal vector (Miller)"


I suggest trying to keep wording simpler:

Suggested change

"The Viewing direction must a real space vector (Vector3d), not a cystal vector (Miller)"

"The sample direction must be a sample vector, not a crystal vector"

hakonanes · 2026-03-08T07:44:12Z

orix/tests/test_plot/test_direction_color_keys.py

+        rgb = rgb_key.direction2color(direction=v)
+        rgb = rgb_key.direction2color(direction=m1)
+        with pytest.raises(ValueError, match="'direction' has a Laue group"):
+            rgb = rgb_key.direction2color(direction=m2)


Suggested change

rgb = rgb_key.direction2color(direction=v)

rgb = rgb_key.direction2color(direction=m1)

with pytest.raises(ValueError, match="'direction' has a Laue group"):

rgb = rgb_key.direction2color(direction=m2)

rgb1 = rgb_key.direction2color(direction=v)

rgb2 = rgb_key.direction2color(direction=m1)

assert np.allclose(rgb1, rgb2)

with pytest.raises(ValueError, match="'direction' has a Laue group"):

rgb_key.direction2color(direction=m2)

hakonanes · 2026-03-08T07:47:02Z

orix/plot/orientation_color_keys/ipf_color_key_tsl.py

+                )
+            if not isinstance(direction, Vector3d):
+                try:
+                    direction = Vector3d(np.asanyarray(direction))


Since we allow this, we need to update the signature type hints. I suggest the following:

def __init__( self, symmetry: Symmetry, direction: Vector3d | list | tuple | None = None ) -> None:

hakonanes · 2026-03-08T07:48:39Z

orix/plot/orientation_color_keys/ipf_color_key_tsl.py

+                except:
+                    raise ValueError("'direction' cannot be interpreted as a Vector3d")


This error catching should be narrowed, and the thrown error communicated to the user:

Suggested change

except:

raise ValueError("'direction' cannot be interpreted as a Vector3d")

except DimensionError as err:

raise ValueError("'direction' cannot be interpreted as a Vector3d") from err

hakonanes · 2026-03-08T07:49:08Z

orix/plot/orientation_color_keys/ipf_color_key_tsl.py

+                except:
+                    raise ValueError("'direction' cannot be interpreted as a Vector3d")
+            if direction.size != 1:
+                raise ValueError("'direction' only accepts a single vector as an input")


Suggested change

raise ValueError("'direction' only accepts a single vector as an input")

raise ValueError("Only one sample direction can be given")

hakonanes · 2026-03-08T07:50:27Z

CHANGELOG.rst

+- ``IPFColorKeyTSL`` will now only accept a single ``Vector3D`` as the `direction` setting
+   during creation.


Suggested change

- ``IPFColorKeyTSL`` will now only accept a single ``Vector3D`` as the `direction` setting

during creation.

- ``IPFColorKeyTSL`` now only accepts a single ``Vector3d`` as the ``direction``.

hakonanes · 2026-03-08T07:54:34Z

Also, how about merging to main and making a patch release 0.14.3 right away? It is a bug after all.

argerlt added 4 commits March 4, 2026 16:23

allow miller indicies as inputs to direction2color (pyxem#638)

dd0b729

sanitize inputs to ipf_color_key_tsl to require Vector3d input

5d50686

Add tests for orientation2color and direction2color

2217cf0

Add some additional error messages and update changelog

8b4e972

[pre-commit.ci] auto fixes from pre-commit.com hooks

fa1aac1

for more information, see https://pre-commit.ci

argerlt requested a review from hakonanes March 5, 2026 01:18

hakonanes added the bug Something isn't working label Mar 5, 2026

hakonanes added this to the v0.15.0 milestone Mar 5, 2026

Merge branch 'sanitize_direction2color' of https://github.com/argerlt…

3732281

…/orix into sanitize_direction2color

[pre-commit.ci] auto fixes from pre-commit.com hooks

f064009

for more information, see https://pre-commit.ci

hakonanes requested changes Mar 8, 2026

View reviewed changes

		h_laue = direction.phase.point_group.laue.name
		if h_laue == self.symmetry.name:

	"The Viewing direction must a real space vector (Vector3d), not a cystal vector (Miller)"
	"The sample direction must be a sample vector, not a crystal vector"

		except:
		raise ValueError("'direction' cannot be interpreted as a Vector3d")

-                except:
-                    raise ValueError("'direction' cannot be interpreted as a Vector3d")
+                except DimensionError as err:
+                    raise ValueError("'direction' cannot be interpreted as a Vector3d") from err

	raise ValueError("'direction' only accepts a single vector as an input")
	raise ValueError("Only one sample direction can be given")

		- ``IPFColorKeyTSL`` will now only accept a single ``Vector3D`` as the `direction` setting
		during creation.

	- ``IPFColorKeyTSL`` will now only accept a single ``Vector3D`` as the `direction` setting
	during creation.
	- ``IPFColorKeyTSL`` now only accepts a single ``Vector3d`` as the ``direction``.

Conversation

argerlt commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of the change

Progress of the PR

Minimal example of the bug fix or new feature

For reviewers

Uh oh!

argerlt commented Mar 5, 2026

Uh oh!

hakonanes commented Mar 5, 2026

Uh oh!

argerlt commented Mar 5, 2026

Uh oh!

argerlt commented Mar 6, 2026

Uh oh!

hakonanes left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hakonanes commented Mar 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

argerlt commented Mar 5, 2026 •

edited

Loading