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

Format templ_attr.cif #407

Closed
wants to merge 2 commits into from
Closed

Conversation

rowlesmr
Copy link
Collaborator

@rowlesmr rowlesmr commented Jun 2, 2023

From #386

I've applied the same (hopefully) formatting requirements for cif_core.dic to templ_attr.cif, wrt which columns different things should be in.

I've also reformatted the descriptions to go up to the 80 character mark. Not completely sure if this is a good idea or not.

Feel free to revert this commit. I'm not completely wedded to it.
@rowlesmr
Copy link
Collaborator Author

rowlesmr commented Jun 2, 2023

I'm leaving templ_enum.cif until after all the indexing stuff, default enumeration values, and reference info is sorted.

@vaitkus
Copy link
Collaborator

vaitkus commented Jun 3, 2023

Is is possible that you made these changes in your add_beta_ADPs (https://github.com/rowlesmr/cif_core/tree/add_beta_ADPs) branch instead of main? It seems that the PR also includes some non-formating changes (e.g. addition of the save_aniso_betaij). We should not merge it in this form.

On a related note, I am not sure it is worth making these changes by hand. @jamesrhester may correct me on this, but I think he already has software which brings the dictionaries up-to-code automatically. I think we should go with the automated approach if possible since it is less error-prone.

@rowlesmr
Copy link
Collaborator Author

rowlesmr commented Jun 3, 2023

Yeah. I don't know what I did there. I was switching between branches a lot yesterday.

@rowlesmr rowlesmr closed this Jun 3, 2023
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