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

Add delete_atoms method and use bonding info for add_hatoms SO107 #178

Merged
merged 8 commits into from
Dec 20, 2024

Conversation

dormrod
Copy link
Contributor

@dormrod dormrod commented Nov 25, 2024

Description

Two small changes to Molecule:

  • delete_atoms: add a method to delete multiple atoms in a single call. This uses "partial success" so that all passed atoms will attempted to be deleted, even if one of them fails.

  • add_hatoms: use a .in file to amsprep instead of a .xyz so bonding information is respected when adding h-atoms. Also perform a guess bonds on the output, if there were bonds on the original molecule.

mol/molecule.py Outdated Show resolved Hide resolved
@dormrod dormrod force-pushed the DavidOrmrodMorley/molecule_add_hatoms_fix branch 2 times, most recently from f83dda7 to 995b199 Compare December 11, 2024 15:17
@dormrod dormrod requested a review from GiulioIlBen December 12, 2024 13:02
@dormrod dormrod force-pushed the DavidOrmrodMorley/molecule_add_hatoms_fix branch from 995b199 to 384a7dd Compare December 16, 2024 14:37
Copy link
Contributor

@GiulioIlBen GiulioIlBen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a notebook for showing the add_hatoms and I decided to show also how to visualize molecules in the same notebook.

It would be nice to obtain the plt.Axes of the molecule as visualized in the notebook, but I couldn't figure out how to do it. Since this is off-topic, I think we can leave the notebook as it is.

Copy link
Contributor

@GiulioIlBen GiulioIlBen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it is good now.

@GiulioIlBen GiulioIlBen merged commit aff62d7 into trunk Dec 20, 2024
17 checks passed
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