Improve DiffractionSimulation

[din14970]

---

name: Fix DiffractionSimulation
about: see #174

---

Release Notes
Reworked the way DiffractionSimulation behaves, thereby fixing issues setting properties like coordinates, indices and intensities.

What does this PR do? Please describe and/or link to an open issue.
This PR aims to fix #174 but also reworks the class a bit

The changes mainly pertain to the property/setter behavior of the coordinates, indices and intensities attributes. Previously DiffractionSimulation could exist as an empty shell without any diffraction spots, and coordinates, indices and intensities could be set to anything. This can cause all kinds of issues with the getters, that relied on _coordinates, etc. to be either all arrays of the same size or None. But this was not enforced anywhere, plus one ran into trouble with the with_direct_beam masking.

I have changed coordinates to a mandatory argument in the __init__. My reasoning is that a diffraction pattern without any spots doesn't make much sense. A completely empty array can still be passed. Indices and intensities can be left empty but appropriately sized arrays of zeros will be automatically created based on the size of coordinates. In the init method, the shape of all these arrays is verified, and _coordinates, _indices and _intensities are set based on what is passed in. The getters and setters of indices intensities and coordinates can then be substantially simplified, both rely on self.direct_beam_mask and stay consistent in shape amongst each other since I use numpy slicing to update the values. Changing the number of spots is no longer allowed, for this one should simply create a new DiffractionSimulation. What can be done instead now is slicing into the pattern, for example

sim[sim.intensities > 8]

in order to filter out spots based on some condition. This makes sense as long as coordinates, indices, and intensities are the same size. note that on a slice like this, information about the direct beam is lost if it was already "filtered out" by with_direct_beam

Other changes I've made were mainly as a consequence of the previous changes and also make sense in my opinion. calibrated_coordinates only makes sense in the x-y plane, there is no reason it should return the z-coordinate since there is no calibration value for it. One might consider renaming this detector_coordinates?

30/09/21

I have tried to clean up this class a bit more and add a couple more features:

• allow for "adding" and "extending" patterns with other patterns, effectively concatenating coordinate, index and intensity arrays. Basically

sim1 + sim2

Will produce another DiffractionSimulation object with the combined spots of the original two. With the extend method, spots can be added from another pattern in-place, just like it exists in ASE.

• added rotation, mirroring and translation arguments for plotting, creating a mask and creating a gaussian image of the pattern. It is convenient to be able to rotate, flip and shift the templates easily. I also added a method to rotate, shift, mirror the template spot coordinates in-place.
• The method for creating a diffraction pattern image was reworked to allow for non-square images
• After the issue in diffsims is not giving this user the diffraction pattern they were expecting (from MMSE). #176 , I think it is unreasonable to set calibration to 1 by default. Instead I set it to none by default and whenever a user tries to access calibrated_coordinates or a method that depends on it, an exception is raised telling the user the calibration should be set.

Describe alternatives you've considered

Are there any known issues? Do you need help?

Work in progress?

• add testing for getitem
• update changelog

For reviewers

• Check that the PR title is short, concise, and will make sense 1 year
  later.
• Check that new functions are imported in corresponding __init__.py.
• Check that new features, API changes, and deprecations are mentioned in
  the unreleased section in CHANGELOG.md.

[din14970]

Coveralls is not accessible at the moment which seems to be the reason why CI is failing

[hakonanes]

FYI, Coveralls now works!

Edit: At least the workflows don't fail. See the reduced coverage.

[din14970]

Think this should be ok for review, if you agree with the features I've added I'll update the changelog

[hakonanes]

I'll have a look tomorrow, but since I haven't used this part of diffsims, someone else who has should have a look as well before merging.

[hakonanes]

Looks like solid additions! I've made general comments and suggestions on the structure of added functionality, which I believe could make it less error prone and easier to maintainable.

(Would be great to have a user guide notebook or something to see the added functionality in action, but that is for another time, and other people perhaps.)

[hakonanes]

Looks good! I'll leave it up to @pc494 or @dnjohnstone to merge.

[pc494]

Thanks for the contribution @din14970 and the review @hakonanes, apologies for my slowness.