Skip to content

Dependencies updates, and better handling of hydrogens, stereochemistry, and aromaticity - #67

Merged
mmzdouc merged 47 commits into
mainfrom
adafede-patch
Jul 17, 2026
Merged

mmzdouc merged 47 commits into
mainfrom
adafede-patch

Conversation

@Adafede

@Adafede Adafede commented Jul 14, 2026

Copy link
Copy Markdown
Member

Hi @mmzdouc,

Hopefully this should fix mite-standard/mite_data#533
and the issue you mentioned in the Slack channel

Moreover, it superseeds #65.

I still want to update rdkit and manually fix entries as per #62 but will wait for your input before.

@Adafede Adafede changed the title https://github.com/mite-standard/mite_data/issues/533 Dependencies updates, and better handling of hydrogens, stereochemistry, and aromaticity Jul 15, 2026
@mmzdouc

mmzdouc commented Jul 15, 2026

Copy link
Copy Markdown
Member

@Adafede thank you for the PR! I have added the test - not passing yet.
Since we are at it, I would propose to remove the network calls to UniProt/NCBI/Wikidata/Rhea to speed up validation - they are throwing warnings only, which are usually ignored anyway. I have already re-implemented them on the frontend (API call to UniProt/NCBI to check the accession before submission, e.g. here) and during release preparation (see here)

@mmzdouc

mmzdouc commented Jul 15, 2026

Copy link
Copy Markdown
Member

(I will push another commit in a minute)

@Adafede

Adafede commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

Passing now 👍🏼

@mmzdouc

mmzdouc commented Jul 15, 2026

Copy link
Copy Markdown
Member

Commit caad15f cuts run time enormoulsy; test are now passing in ca 1 sec (was 35 secs before)

@mmzdouc

mmzdouc commented Jul 15, 2026

Copy link
Copy Markdown
Member

@Adafede I will update some cleanup rules for ketcher and run the new version of mite_extras against our current entries to verify backward compatibility; after that, we are ready to merge I think!

@Adafede

Adafede commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

Let me know if rdkit update should be included or not within this one... if the amount of broken entries will be acceptable or not (anyway, I think the sooner the better or they will just get more)

@mmzdouc

mmzdouc commented Jul 15, 2026

Copy link
Copy Markdown
Member

@Adafede removing R,S stereochemistry for reaction SMARTS seems to work nicely with

echo $smarts_string | tr -d "@"

@Adafede

Adafede commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

@mmzdouc Think I finally got something slightly cleaner...

@mmzdouc

mmzdouc commented Jul 16, 2026

Copy link
Copy Markdown
Member

@Adafede wow all entries are passing 🤩
Out of curiosity, I updated rdkit to the latest version and ran the validation again. 66 entries are failing, mostly due to stereochemistry (expected product vs generated product). For the ones that I have looked at, it seems that the exact enantiomere was produced. I have added a test for one, perhaps you can take another look? I am still in favor of simply flattening the entires and moving on, but if they can be easily salvaged, that would be great, too!

@Adafede

Adafede commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

@mmzdouc

I just made the new version return a warning about stereo mismatches, will help everyone fixing these, tell me if this fits what you expected!

@mmzdouc

mmzdouc commented Jul 17, 2026

Copy link
Copy Markdown
Member

@Adafede great workaround - I like that! I have made some small adujstments to the error text. I would rather not allow people to skip the validation step and I replaced this statement with a shoutout for people to get in touch with the maintainers.
I just saw that a new version of rdkit was released and I ran the validation script. The only entry that does not pass is MITE0000346 which is retired anyway.
For me, this is release-ready. I will do an update of the full stack today.

@mmzdouc mmzdouc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@mmzdouc

mmzdouc commented Jul 17, 2026

Copy link
Copy Markdown
Member

@Adafede we should probably unpin the Python version? We are not bound by rdkit anymore. I am pinning the Python version in the mite_web docker container building anyway

@mmzdouc

mmzdouc commented Jul 17, 2026

Copy link
Copy Markdown
Member

@Adafede I did a quick update of the CHANGELOG. Feel free to replace the Misc changes with a brief description of your most important changes.

@mmzdouc

mmzdouc commented Jul 17, 2026

Copy link
Copy Markdown
Member

@Adafede I fully replaced pre-commit with prek. Will go on and merge the PR. Thanks for all the great work!

@mmzdouc
mmzdouc merged commit f0636fe into main Jul 17, 2026
4 checks passed
@mmzdouc
mmzdouc deleted the adafede-patch branch July 17, 2026 07:47
@mmzdouc mmzdouc mentioned this pull request Jul 17, 2026
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.

Update troubleshooting

2 participants