connorcoley / rdchiral

Wrapper for RDKit's RunReactants to improve stereochemistry handling
MIT License
151 stars 50 forks source link

Allow for custom atom mapping in reactants when running reaction from SMARTS #47

Closed ykevu closed 10 months ago

ykevu commented 10 months ago

By default, initialization of the rdchiralReactants class automatically overwrites any specified atom mapping in the input reactant_smiles to a range from 0 to the total number of atoms. However, there are cases where one would want to specify atom mappings (i.e. in a global context across multiple reactions).

For example, in the case where the user specifies:

"smarts": "[C;R:10][O;R:2][Xe;R:3][O;R:4][C;R:5]>>[C:10][O:2].[O;R:4][C;R:5]",
"smiles": "[C:10]1[C:9][O:8][Xe:7][O:6]1",

We want

"expected": ["[C:10]([C:9][OH:8])[OH:6]"],

This PR adds a flag custom_reactant_mapping which defaults to False such that behavior is still the same in all cases unless the user wants to keep a custom atom mapping for their reactant. Note that in doing so, all atoms must have a specified map number, or a key error will be thrown when running the reaction. Some random test cases were pulled from test_rdchiral_cases.json and given custom atom mappings to ensure that the functionality of running reactions still behaves as expected.

ykevu commented 10 months ago

@connorcoley I don't have write access, would you mind merging this or should someone else review it as well?