Ensure atom positions are expressed in the new basis when initializing Phase with structure

[viljarjf]

Fixes (part of) pyxem/diffsims#203

Description of the change

When initializing a Phase with a diffpy.structure.Structure, the lattice is transformed to orix's standard coordinate system. However, the atoms are still expressed as if they were in the old lattice.
By taking the cartesian positions of the atoms before changing the lattice, and using those to set the new positions, the change of basis is accounted for.

All tests pass, but I don't think this was tested for previously. It probably should.

Progress of the PR

• 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

from diffpy.structure import lattice, atom, Structure
from orix.crystal_map import Phase
import numpy as np

# Graphite
latt = lattice.Lattice(2.464, 2.464, 6.711, 90, 90, 120)
atoms = [atom.Atom(atype='C', xyz=[0.0, 0.0, 0.25], lattice=latt),
         atom.Atom(atype='C', xyz=[0.0, 0.0, 0.75], lattice=latt),
         atom.Atom(atype='C', xyz=[1/3, 2/3, 0.25], lattice=latt),
         atom.Atom(atype='C', xyz=[2/3, 1/3, 0.75], lattice=latt),
         ]
structure = Structure(atoms=atoms, lattice=latt)

phase = Phase("Graphite", point_group="6/mmm",  structure=structure)

# The lattice has been transformed
assert not np.allclose(structure.lattice.base, phase.structure.lattice.base)

for i in range(len(atoms)):
    print("xyz-basis:  ", structure[i])
    print("phase-basis:", phase.structure[i])
    print()

Output before:

xyz-basis:   C    0.000000 0.000000 0.250000 1.0000
phase-basis: C    0.000000 0.000000 0.250000 1.0000

xyz-basis:   C    0.000000 0.000000 0.750000 1.0000
phase-basis: C    0.000000 0.000000 0.750000 1.0000

xyz-basis:   C    0.333333 0.666667 0.250000 1.0000
phase-basis: C    0.333333 0.666667 0.250000 1.0000

xyz-basis:   C    0.666667 0.333333 0.750000 1.0000
phase-basis: C    0.666667 0.333333 0.750000 1.0000

Output with this PR:

xyz-basis:   C    0.000000 0.000000 0.250000 1.0000
phase-basis: C    0.000000 0.000000 0.250000 1.0000

xyz-basis:   C    0.000000 0.000000 0.750000 1.0000
phase-basis: C    0.000000 0.000000 0.750000 1.0000

xyz-basis:   C    0.333333 0.666667 0.250000 1.0000
phase-basis: C    0.577350 0.577350 0.250000 1.0000

xyz-basis:   C    0.666667 0.333333 0.750000 1.0000
phase-basis: C    0.577350 -0.000000 0.750000 1.0000

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.

[pc494]

@viljarjf would you be comfortable writing a test case for this? If so, I'll wait for that and then get my crystallography hat on to check this works as it needs to.

[CSSFrancis]

@viljarjf I would just add the example in as a test. It at least makes sure that we explicitly know what basis we are working in and if for some reason diffpy changes then we would know.

[viljarjf]

I added a test now, a cubic lattice passes with the old code but the two non-cubic lattices failed. All three pass with the fix

[CSSFrancis]

@viljarjf Just a note that you need to run both black and isort, let me know if you need help doing that. @pc494 do you want to allow pre-commit similar to pyxem so we can just auto-fix files. I think that is much easier.

As far as the failing tests go. They seem unrelated. @pc494 You can rerun the tests if you would like and it might allow them to pass otherwise there is a different bug.

[pc494]

Both,

I'm going to check this out today to refactor the testing a little bit and I'll get isort + black etc run then as well.

[CSSFrancis]

@pc494 This needs to be added to the Changelog as well

[pc494]

pre-commit.ci autofix

[pc494]

Sorry @viljarjf, @CSSFrancis I got very lost in the tests here and, given the pace of change in orix I wanted to be super sure on what was going on. Does the refactor I've written make sense?

[pc494]

This seems okay to me, but I'm mainly taking the correctness of this on faith from @hakonanes in pyxem/diffsims#203 (comment)

[CSSFrancis]

@hakonanes I tested this vs Kikchipy and the tests appear to be passing.

[CSSFrancis]

@pc494 any idea why the documentation is failing?

[pc494]

I'm going to tidy this up with a view to merge later today. Hopefully, the docs will figure themselves out,..

[pc494]

pre-commit.ci autofix

[pc494]

Given the current state of play, I am going to merge this, with new issues raised about both the RTD build and the contributors list.