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.

Sign up for GitHub

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

Jump to bottom

'Adjust Hydrogens' sometimes fails #678

Closed
grsousajunior opened this issue Jul 16, 2021 · 5 comments · Fixed by #1112
Closed

'Adjust Hydrogens' sometimes fails #678

grsousajunior opened this issue Jul 16, 2021 · 5 comments · Fixed by #1112
Labels
bug
Projects
Release 1.96 Release 1.98

Comments

@grsousajunior
Copy link
Contributor

Avogadro version: 1.94.0

Desktop version:

  • OS: Microsoft Windows 64-bit operating system, x64-based processor
  • Version: 10.0.19043.1083

Describe the bug
Using the Pencil Tool to draw a cyclopropane molecule, when connecting the last and first carbon atoms, the last one appears without hydrogen atoms.

Odd behavior on a monocyclic molecule

This odd behavior happens with any monocyclic and bicyclic molecule.

Odd behavior on a bicyclic molecule

However, using Nitrogen as last atom, everything works fine.

Odd behavior using Nitrogen as last atom
@mquevill
Copy link
Contributor

I can also confirm this happens on Linux Mint too, built from master.

Oddly enough, some bicyclic cases work right...

Starting from incorrect cyclobutane:
image
Adding a methyl group (which corrects the hydrogen count):
image
Connecting existing atoms (making it bicyclic):
image

@ghutchis ghutchis added the bug label Aug 13, 2021
@mquevill mquevill mentioned this issue Sep 10, 2021
Closed
@ghutchis ghutchis changed the title Problem with 'Adjust Hydrogens' on cyclic molecules 'Adjust Hydrogens' sometimes fails Sep 10, 2021
@ghutchis ghutchis mentioned this issue Sep 10, 2021
Closed
@ghutchis ghutchis added this to To do in Release 1.96 May 2, 2022
@ghutchis ghutchis added this to To do in Release 1.98 Jul 27, 2022
ghutchis added a commit to ghutchis/avogadrolibs that referenced this issue Aug 18, 2022
@ghutchis
Make sure to add / adjust valence in the correct order
d975b14 
Also make sure to retain unique id for batch adjustment.
Fix OpenChemistry#678 in my testing

Signed-off-by: Geoff Hutchison <geoff.hutchison@gmail.com>
@ghutchis ghutchis mentioned this issue Aug 18, 2022
Merged
@github-actions
Copy link
Contributor

Here are the build results
Avogadro2.AppImage
Ubuntu-2004.tar.gz
Win64.exe
Artifacts will only be retained for 90 days.

Release 1.98 automation moved this from To do to Done Aug 18, 2022
@ghutchis
Copy link
Member

If you have a chance to check the latest master with #1112 I'd appreciate it. I can confirm it fixed this bug in my testing. Sorry it took a while to track down.

@github-actions
Copy link
Contributor

Here are the build results
Avogadro2.AppImage
Ubuntu-2004.tar.gz
Win64.exe
Artifacts will only be retained for 90 days.

@mquevill
Copy link
Contributor

Tested at e4b94fa, and all the test cases shown here act correctly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
No open projects
Release 1.96
To do
Release 1.98
  
Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make sure to add / adjust valence in the correct order ghutchis/avogadrolibs
3 participants
@ghutchis @mquevill @grsousajunior