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

Update definitions of arbitrary id data items. #50

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

vaitkus
Copy link
Collaborator

@vaitkus vaitkus commented Sep 11, 2023

This PR updates id definitions so they match those in the CIF_CORE dictionary.

Note, that in some cases the assigned source values differ from the default ones, therefore this might conflict with PR #46.

@jamesrhester
Copy link
Contributor

I'm going to guess that existing code has not taken advantage of the freedom offered by the "Text" type and so we can restrict to "Word" type. I would prefer that we bundle these small changes into an email that @brantonc can send to the relevant software authors to double-check that they have no issues.

@jamesrhester jamesrhester added the consultation Change put out for broader consultation label Sep 13, 2023
@vaitkus
Copy link
Collaborator Author

vaitkus commented Sep 13, 2023

Ok, so this particular change can be summarised as:

The following identifier data items would no longer be allowed to have ASCII whitespaces in their values:

  • _atom_site_moment_Fourier.id
  • _atom_site_moment_Fourier_param.id
  • _parent_propagation_vector.id
  • _space_group_magn_ssg_transforms.id
  • _space_group_magn_transforms.id
  • _space_group_symop_magn_centering.id
  • _space_group_symop_magn_og_centering.id
  • _space_group_symop_magn_ssg_centering.id
  • _space_group_symop_magn_ssg_operation.id

@brantonc
Copy link
Collaborator

I support this change. I'm not aware of any software authors for whom this would be a problem.

@jamesrhester jamesrhester merged commit 0e1c45d into COMCIFS:main Jan 23, 2024
2 checks passed
@vaitkus vaitkus deleted the update-id-definitions branch January 23, 2024 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consultation Change put out for broader consultation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants