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 the _atom_site_fract.symmform and _atom_site_aniso.symmform data items #473

Merged
merged 4 commits into from
Feb 22, 2024

Conversation

vaitkus
Copy link
Collaborator

@vaitkus vaitkus commented Jan 18, 2024

This PR adds the _atom_site_fract.symmform and _atom_site_aniso.symmform data items (see discussion in [1]).

The definitions of these items were almost verbatim copied from the document attached in [1] with the following changes:

  • Data item _atom_site_fract.symmform was renamed to _atom_site.fract_symmform (changed the placement of the dot). The category of this item was also changed from ATOM_SITE_FRACT to ATOM_SITE.
  • The human-readable definition of _atom_site_aniso.symmform was changed to use the term "anisotropic displacement parameters" instead of "anisotropic thermal ellipsoid".
  • Both items were assigned the "Assigned" _type.source.
  • Both items were assigned the "Encode" _type.purpose. (I assume here that they are intended to be machine-parsable).

Before merging this, please also note that @jamesrhester raised some reasonable concerns in [1] (e.g., the same information might be better conveyed using a CIF2/DDLm list). However, since the _atom_site_fract.symmform is already used by some pieces of software and already appears in the wild, I suggest that we retain the semantics of these two data items and introduce additional list items (now or later) if so desired. How does that sound?

[1] COMCIFS/magnetic_dic#59

Copy link
Contributor

@jamesrhester jamesrhester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted elsewhere, these are better presented as lists but given that they are already in use we should capture them in this form as well. I will raise an issue that a list-version should be defined.

@vaitkus
Copy link
Collaborator Author

vaitkus commented Feb 22, 2024

Thank you for the review.

The issue that deals with the list form of these items is #484, the same will probably have to be done in the magnetic CIF dictionary as well.

@vaitkus vaitkus merged commit 00815d6 into COMCIFS:master Feb 22, 2024
3 checks passed
@vaitkus vaitkus deleted the add-atom-site-symmform-items branch February 22, 2024 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants