Conversation
amyjaynethompson
left a comment
amyjaynethompson
left a comment
There was a problem hiding this comment.
Thanks for making these changes - there's some really useful looking stuff here :)
Have left a few comments mostly for general code structure stuff - you definitely have access to better test data than I do at the moment, so I trust it's working as you intended :)
If there are any specific commits you would like to go on main, you should be able to:
- switch to main
- git pull to update
- checkout new branch (ie main_updates)
- git cherry pick
If the changes for main are not neatly in separated commits then it might be an annoying manual copy/paste job... Might also encounter conflicts that need resolving, but see how you go :)
| logging.info(f"DEBUG: Running centroids with location={location}") | ||
| logging.info(f"DEBUG: centroid_1_atoms={self.cfg['centroid_1_atoms']}") | ||
| logging.info(f"DEBUG: centroid_2_atoms={self.cfg['centroid_2_atoms']}") | ||
| logging.info(f"DEBUG: centroid_1_symmetry={self.cfg.get('centroid_1_symmetry')}") | ||
| logging.info(f"DEBUG: centroid_2_symmetry={self.cfg.get('centroid_2_symmetry')}") |
There was a problem hiding this comment.
Per previous comment - if this logging is put inside the centroid_distance_analysis function you can avoid duplicate code.
| or None if no atoms were found | ||
| """ | ||
|
|
||
| if isinstance(atom_names, str): |
There was a problem hiding this comment.
Throw a warning or error if this is not the case....????
| self.cfg, self.sys = self.config.yaml_reload(self.test_mode) | ||
|
|
||
| def ref_edit(self, ins: str, MPLA_atoms: str) -> None: | ||
| def ref_edit(self, ins: str, MPLA_atoms) -> None: |
There was a problem hiding this comment.
Need appropriate type hint for MPLA_atoms
There was a problem hiding this comment.
Yay for updating tests :)
|
changes should be done... |
amyjaynethompson
left a comment
amyjaynethompson
left a comment
There was a problem hiding this comment.
Thanks for addressing the comments - should be ok to go into dev now :)
2 participants
Here is the more substantial update and it seems to work... we will continue to test but all seems ok.
Basically I have pulled out the maths out of a few things to make a new crystal_maths utility and then I have used that to make analysis of multiple planes possible.
Then I've introduced centroid-centroid measurements - even taking symmetry codes. There is a module and pipeline. I am pulling the coordinates out of the .lst files so there is now a .lst parser. Thanks to Claude for writing that!
I've also fixed some of the graphing for hbonds.
Things to think about in the future.... should we do a centroid/rotation calc from cif? any other analysis we need :)