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

SmilesParser is not idempotent #92

Closed
targos opened this issue Apr 5, 2023 · 3 comments
Closed

SmilesParser is not idempotent #92

targos opened this issue Apr 5, 2023 · 3 comments

Comments

@targos
Copy link
Contributor

targos commented Apr 5, 2023

Once it's been called with readStereoFeatures = true, subsequent calls with readStereoFeatures = false don't work as expected:

SmilesParser parser = new SmilesParser();
StereoMolecule molecule = new StereoMolecule(0, 0);
String smiles = "Cl/C=C/Br";

parser.parse(molecule, smiles.getBytes(), true, false);
System.out.println(molecule.getIDCode()); // gC`DAbZHRVX@

parser.parse(molecule, smiles.getBytes(), true, true);
System.out.println(molecule.getIDCode()); // gC`DAbZHRVXRP

parser.parse(molecule, smiles.getBytes(), true, false);
System.out.println(molecule.getIDCode()); // gC`DAbZHRVXRP

Moreover, if we call it with createCoordinates = false, it will always print gC`DAbZHRVXRP, regardless of the value of readStereoFeatures.

@targos
Copy link
Contributor Author

targos commented Dec 22, 2023

@thsa What do you think?

@thsa
Copy link
Contributor

thsa commented May 1, 2024

@targos Hi Michael, sorry for the late response. I didn't notice the question. I have just fixed it. In this situation the StereoMolecule was re-using the Canonizer from previous runs and taking parity information from there.

Concerning createCoordinates = false: You cannot use molecule.getIDCode(), if the Molecule has valid parities and does not contain atom coordinates. Thus, after Smiles parsing with createCoordinates = false we have this sutuation and it will always print 'null' now. molecule.getIDCode() is a convenient method, which only works, if a Canonizer was created and used during the ensureHelperArrays() call.

@thsa thsa closed this as completed May 1, 2024
@targos
Copy link
Contributor Author

targos commented May 2, 2024

Thank you for the fix.

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