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

To Do: Hydrogen accounting via bond orders #14

Closed
fgrunewald opened this issue Jul 8, 2024 · 6 comments
Closed

To Do: Hydrogen accounting via bond orders #14

fgrunewald opened this issue Jul 8, 2024 · 6 comments

Comments

@fgrunewald
Copy link
Collaborator

At the moment the code applies several pysmiles functions to add the right number of hydrogen atoms. In principle we should be able to just get that from the correct bond orders, however, aromaticity is kind of preventing this. The current code works but we should clean this up at some point. Related to PR #12.

@pckroon
Copy link
Collaborator

pckroon commented Jul 8, 2024

I think the correct solution is to count aromatic bond orders as 1, and deduct 1 for the valency from each aromatic atom.

@fgrunewald
Copy link
Collaborator Author

Unfortunetly not. Take p-cresol as example: {[#SN3a]1[#TC5]2[#SN2a][#TC5]12}.{#SN3a=[$][$]c[N+](=O)[O-],#TC5=[$]cc[$],#SN2a=[$][$]cOC}

The carbon in the SN3a/SN2a fragment has an hcount of 3 and then from two aromatic bonds so -2. It ends up with CH which coincidentially is valid for this molecule.

pcresol

@pckroon
Copy link
Collaborator

pckroon commented Jul 8, 2024

But, those carbons have valency 4. Subtract 3 bonds (of which 2 aromatic), subtract 1 because it's aromatic, leaves 0, right?

@fgrunewald
Copy link
Collaborator Author

yes but not in the place where you're thinking. because during the resolve process the valency is not known

@pckroon
Copy link
Collaborator

pckroon commented Jul 8, 2024

Fixing the hydrogens should be the very very last thing right?
Or is this a pysmiles issue?

@fgrunewald
Copy link
Collaborator Author

this has been solved with #29

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

No branches or pull requests

2 participants