orix icon indicating copy to clipboard operation
orix copied to clipboard

add to_dict method to crystal_map to allow for easy extraction of tho…

Open SteffenBrinckmann opened this issue 2 years ago • 4 comments

add to_dict method to crystal_map to allow for easy extraction of those results that would be printed with repr

Description of the change

Progress of the PR

  • [X] Docstrings for all functions
  • [ ] Unit tests with pytest for all lines -> most tests were successful; some failed because of some 'url.requests' issue.
  • [ ] Clean code style by running black via pre-commit -> copied existing code: hence style should be ok (I don't have black installed and have no experience of using it)

Minimal example of the bug fix or new feature

>>> from orix.io import load
>>> datadir = 'data/04/'
>>> fname = 'sdss_ferrite_austenite.ang' #.ang file produced by EMsoft
>>> cm = load(datadir + fname)
>>> metadata = cm.to_dict()

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.

SteffenBrinckmann avatar Aug 29 '23 09:08 SteffenBrinckmann

Thank you for this suggestion, @SteffenBrinckmann.

A public method to extract all contents of a crystal map is a good idea. We already use such a private function in the orix HDF5 writer:

https://github.com/pyxem/orix/blob/2e6f783e07c750f09553bec6fc51f5dfa9abae96/orix/io/plugins/orix_hdf5.py#L248-L298

This function returns all contents of a crystal map, including the rotations. It doesn't calculate the phase fractions for you, though. Can you explain what you need this information for, specifically?

I think making the mentioned private function a public crystal map method is the best solution. If you really need the phase fractions as well, we can add this to the dictionary if a boolean keyword is true, for example. What do you think?

hakonanes avatar Aug 29 '23 20:08 hakonanes

hey @hakonanes

  • The fractions are not that difficult to evaluate and does warrant an extra function.
  • I did not know about that function existing, but yes one function makes more sense.
  • I would just suggest to not have the function in a writer (it is not about a writer) but it is for interaction, so a more general location would make more sense. (I would have never guessed the function being in the hdf writer).

best, Steffen

SteffenBrinckmann avatar Sep 20 '23 09:09 SteffenBrinckmann

I think that @SteffenBrinckmann has a good idea. If the function is private it would be good to add this function to the CrystalMap object. Something like a function _to_dictionary would allow it to be saved by hyperspy signals as well which would be useful.

https://github.com/hyperspy/hyperspy/blob/3b350ff80b68b0928b284e76d3dc0a2b31e6dbf5/hyperspy/misc/utils.py#L524C37-L524C48

@hakonanes @pc494 Thoughts?

CSSFrancis avatar Mar 26 '24 14:03 CSSFrancis

Haven't forgotten about this, but don't have time at the moment. A more thorough discussion about serialization (into dicts, I presume) would be good.

hakonanes avatar May 13 '24 19:05 hakonanes