-
Notifications
You must be signed in to change notification settings - Fork 57
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
adjust logger message #581
base: master
Are you sure you want to change the base?
Conversation
"this shows that these repair functions were not yet run.") | ||
logger.warning("Please first run derive_inchi_from_smiles, derive_smiles_from_inchi and derive_inchikey. " | ||
"The spectrum has partly valid annotations, " | ||
"either these repair functions were not yet run or conversions between the formats failed.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the conversion between the formats fail? That would also explain the removal of some spectra after repairing. I would expect that every annotation that has at least one valid form of annotation, should be able to derive the inchi and inchikey.
@@ -84,9 +84,9 @@ def _check_repairing_is_possible(smiles, inchi, inchikey) -> bool: | |||
elif valid_smiles or valid_inchi or valid_inchikey: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I see this, the valid_inchikey is not enough, since we cannot derive smiles and inchi from inchikey alone. So I would only check elif valid_smiles or valid_inchi:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And above we should also do the same. Instead of if not valid_smiles and not valid_inchi and not valid_inchikey: we should just check if not valid_smiles and not valid_inchi:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@florian-huber See my comments :)
No description provided.