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 to_dict method to crystal_map to allow for easy extraction of tho… #457

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

SteffenBrinckmann
Copy link

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

  • 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.

…se results that would be printed with __repr__
@hakonanes hakonanes added the enhancement New feature or request label Aug 29, 2023
@hakonanes hakonanes added this to the v0.12.0 milestone Aug 29, 2023
@hakonanes
Copy link
Member

hakonanes commented Aug 29, 2023

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:

def crystalmap2dict(crystal_map: CrystalMap, dictionary: Optional[dict] = None) -> dict:
"""Get a dictionary from a :class:`~orix.crystal_map.CrystalMap`
object with ``"data"`` and ``"header"`` keys with values.
Parameters
----------
crystal_map
Crystal map.
dictionary
Dictionary to update with crystal map information. If not given
(default), a new dictionary is created.
Returns
-------
dictionary
Dictionary with crystal map information.
"""
if dictionary is None:
dictionary = {}
# Get data cube coordinates in step size
y, x = [0 if i is None else i for i in [crystal_map._y, crystal_map._x]]
# Get euler angles phi1, Phi, phi2
eulers = crystal_map._rotations.to_euler()
dictionary.update(
{
"data": {
"y": y,
"x": x,
"phi1": eulers[..., 0],
"Phi": eulers[..., 1],
"phi2": eulers[..., 2],
"phase_id": crystal_map._phase_id,
"id": crystal_map._id,
"is_in_data": crystal_map.is_in_data,
},
"header": {
"grid_type": "square",
"ny": y.size if isinstance(y, np.ndarray) else 1,
"nx": x.size if isinstance(x, np.ndarray) else 1,
"y_step": crystal_map.dy,
"x_step": crystal_map.dx,
"rotations_per_point": crystal_map.rotations_per_point,
"scan_unit": crystal_map.scan_unit,
},
}
)
dictionary["data"].update(crystal_map.prop)
dictionary["header"].update({"phases": phaselist2dict(crystal_map.phases)})
return dictionary

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?

@SteffenBrinckmann
Copy link
Author

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

@pc494 pc494 mentioned this pull request Mar 26, 2024
5 tasks
@CSSFrancis
Copy link
Member

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?

@hakonanes hakonanes self-assigned this Mar 29, 2024
@hakonanes hakonanes removed this from the v0.12.0 milestone Apr 10, 2024
@hakonanes
Copy link
Member

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.

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.

3 participants