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

_atom_site_rot_Fourier.id has been removed #1

Open
jamesrhester opened this issue Mar 26, 2020 · 2 comments
Open

_atom_site_rot_Fourier.id has been removed #1

jamesrhester opened this issue Mar 26, 2020 · 2 comments

Comments

@jamesrhester
Copy link
Contributor

_atom_site_rot_Fourier_id exists in the DDL1 version so should be retained in this version, even if not used.

@vaitkus
Copy link
Contributor

vaitkus commented Jul 18, 2023

Furthermore, the _atom_site_rot_Fourier.id item is referenced by the _atom_site_rot_Fourier_param.id data item so it should definitely be defined.

@vaitkus
Copy link
Contributor

vaitkus commented Sep 8, 2023

PR #19 restores the _atom_site_rot_Fourier.id item.

Note, however, that according to the human-readable definition and DDL1 usage examples [1], this particular id was actually a composite key of sorts and not an arbitrary identifier (e.g. Ph1_a1_1 where a1 and 1 refer to values of _atom_site_rot_Fourier_axis and _atom_site_rot_Fourier_wave_vector_seq_id respectively).

Also, the ATOM_SITE_ROT_FOURIER category currently seems a bit strange now:

  • _atom_site_rot_Fourier.atom_site_label is the category key.
  • _atom_site_rot_Fourier.id exists, but is not mandatory.
  • _atom_site_rot_Fourier_param.id references _atom_site_rot_Fourier.id while no items seem to explicitly reference _atom_site_rot_Fourier.atom_site_label.

I have a hunch that there was an effort made to move away from the usage of _atom_site_rot_Fourier.id and some definitions were not fully addressed in this regard.

[1] https://github.com/COMCIFS/DDL1-legacy-dictionaries/blob/28e20dc928790dceb889716d2bed435fc10c4c79/dictionaries/cif_ms.dic#L1951

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

No branches or pull requests

2 participants