-
Notifications
You must be signed in to change notification settings - Fork 1
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
it_coordinate_system_code fix #153
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pull request does not contain a valid label. Please add one of the following labels: ['chore', 'fix', 'bugfix', 'bug', 'enhancement', 'feature', 'dependencies', 'documentation']
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code is tested on one example (Fitting_PD-NEUT-TOF_Si-SEPD.ipynb). It now generates the correct pattern in the case of both possible it_codes for "F d -3 m". The only thing I noticed is that
- when specifying
it_code
in a CIF file via dot notation (_space_group.IT_coordinate_system_code 2
) and reading it via the Job interface, everything seems to work, but the cif output in this case still shows_space_group_IT_coordinate_system_code 1
. - when using the underscore notation in CIF (
_space_group_IT_coordinate_system_code 2
), on the contrary, it shows the correct value of_space_group_IT_coordinate_system_code 2
in the cif output.
Fixes #129 |
I noticed another small issue. If you load a CIF file without the For example, running this: job = ed.Job()
job.add_phase_from_file('data/si.cif')
phase = job.phases['si']
print(phase.space_group.it_coordinate_system_code) leads to (from Cryspy): Several values of it_coordinate_system_code have been defined:
2, 1
The default value has been choosen:'2'. and to (from easydiffraction): <Descriptor 'coordinate-code': 1> Similarly, when assigning a space group manually via |
Hmm I just chose the first element of the settings list for a given spacegroup as returned by gemmi. If there is a way to find out the default setting, I'll be happy to implement it. |
Now we get the settings value based on cryspy choses to be the default. It's the cryspy calculator after all, so importing a method from cryspy makes sense. |
I am still experiencing undesirable behaviour as indicated in my comment above. That is, it seems that when the |
Yes but this is a superfluous message coming from the initial setup of the calculator. |
This adds proper handling of CIF-read it_coordinate_system_code value.
Setting the code in the script also works, using
space_group_IT_coordinate_system_code