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 flag items #47

Merged
merged 4 commits into from
Apr 3, 2024

Conversation

vaitkus
Copy link
Collaborator

@vaitkus vaitkus commented Sep 11, 2023

This PR updates all flag definitions to be more similar to the ones in CIF_CORE. The flag values are made case-sensitive, and are explicitly assigned the State purpose and Assigned source.

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

@jamesrhester
Copy link
Contributor

This might be a bit difficult to change as software authors may already read or write these flags in a case-insensitive way. I'd suggest we leave these alone.

@vaitkus
Copy link
Collaborator Author

vaitkus commented Sep 13, 2023

Oh, I did not see the dictionary did not originate as DDL1 (which would somewhat require it to be case-sensitive for compatibility purposes). However, given that all enumeration values in CIF_CORE are case-sensitive, it would make sense to change it here as well just to be consistent. Maybe we should ask if developers are OK with this change (as suggested in comments of issue #50) or is this particular issue not worth the bother?

@brantonc
Copy link
Collaborator

I have no opinion on this matter.

@jamesrhester
Copy link
Contributor

To resolve properly would require polling the software developers and/or finding examples in the wild of the use of these tags. I've checked CrysFML and CrysFML2008 (underlying code for FullProf) and the latter reads in two of these datanames, but does not yet use the values, and doesn't output them. GSASII does not refer to these anywhere either. Jana code not available. Given that they don't seem widely used yet, happy to merge these changes.

@jamesrhester jamesrhester merged commit 5f407e5 into COMCIFS:main Apr 3, 2024
2 checks passed
@vaitkus vaitkus deleted the update-flag-definitions branch April 3, 2024 06:10
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.

3 participants