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

Pretty-print dictionary according to style rules. #46

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

jamesrhester
Copy link
Contributor

The current "main" branch was re-output using julia_cif_tools/pretty_print.jl , then the result was compared to the source using julia_cif_tools/compare.jl -w -c (no whitespace or case differences considered significant) which found no differences.

@vaitkus
Copy link
Collaborator

vaitkus commented Sep 11, 2023

I also noticed that the some attributes definitions are now explicitly provided instead of relying on their default values (e.g. _type.purpose -> Describe, _type.source -> Assigned). This is necessary to satisfy the style guide requirements, but I was just wandering if the original authors really intended for these specific default values to be used or did they simply left the attribute unfilled. For example, _space_group_symop_magn_ssg_operation.algebraic is assigned the Describe purpose while most likely it should have the Encode purpose (e.g. similar to _space_group_symop.operation_xyz from CIF_CORE).

I am fine with merging the reformatted dictionary as is and then going through the purposes of items later. Alternatively, we may want to go through the default list now and then merge the changes to this PR (or simply rerun the pretty printer).

@jamesrhester
Copy link
Contributor Author

Let's get the pretty-printed version in place, and then make changes relative to it.

@vaitkus
Copy link
Collaborator

vaitkus commented Sep 11, 2023

Ok. Just ignore those PR for now then and I will resubmit once the PP version is in place.

@jamesrhester jamesrhester merged commit 0c25864 into COMCIFS:main Sep 12, 2023
1 check passed
@jamesrhester jamesrhester deleted the pretty_print branch September 12, 2023 04:22
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.

2 participants