Add SHELX hkl file export to dials.export
#1925
Conversation
…artiality (dials#1911) Some corner cases: If the bin critical value is None rather than 0 because of missing all reflections, replace None with 0.0 so that the flex.double call is sane. If no partiality column in reflection table, make sum_partial_reflections a no-op i.e. simply return the input table.
This means the docutils constraint is properly picked up
There was a problem hiding this comment.
Thanks for adding this new feature, it's a great addition. Thank you for also adding tests!
The changes look good overall to me, the only thing that needs addressing is the slight change in _write_ins.
Are you able to rebase your branch on top of the latest changes in dials:main, in the 'files changed' diffs it's showing some unrelated changes from recent commits?
Thanks, James
util/export_shelx.py
Outdated
| wl, *uc.parameters() | ||
| ) | ||
| ) | ||
| if uc_sd is not None: |
There was a problem hiding this comment.
| if uc_sd is not None: | |
| if uc_sd: |
I was testing on scaled datafiles, and L162 caused a crash when trying to format the string as uc_sd was (), because the cell had no parameter sds. This change should stop that.
There was a problem hiding this comment.
Thanks! I've changed that and removed the lines setting uc_sd to None as it shouldn't be required now. I'm still not sure exactly why sometimes unit cell esds disappear - e.g. changing space group from P222 to P212121 with dials.reindex removes them which doesn't make sense to me (but that's outside the scope of this PR).
There was a problem hiding this comment.
changing space group from P222 to P212121 with
dials.reindexremoves them which doesn't make sense to me
In the general case changing the space group invalidates the unit cell esds, but I guess we might need to handle separately the special case where we're not changing the Bravais lattice. Could you make a separate issue for this with an example?
There was a problem hiding this comment.
removed the lines setting uc_sd to None as it shouldn't be required now
🤦 77ff15f fixes bug that I introduced here. Added test too.
There was a problem hiding this comment.
In the general case changing the space group invalidates the unit cell esds, but I guess we might need to handle separately the special case where we're not changing the Bravais lattice. Could you make a separate issue for this with an example?
Done: #1934
|
@huwjenkins you might also want to add your name to the |
|
I've merged in the latest |
I tried to do that before but obviously didn't do it correctly as that's what added the unrelated commits to this branch! Would it be possible to expand the section in |
Thanks 🙂 |
Hmm, yes, I'll have a think about the wording - this would have been fine not to rebase, and rebase is a tool that not a lot of people know well. |
For small molecule work it would be useful to be able to write SHELX hkl format from the output of
dials.scalewithout a detour via MTZ format. This PR adds that functionality.It also:
Writes a stub of a SHELX .ins file with information from the
scaled.exptorrefined_cell.exptfile. I have never used XPREP but I copied the format from hereBy default scales up intensities so the maximum/minimum is 9999.00/-9999.00 maximising precision in f8.2 format.
@dagewa made some comments which I've already addressed.