Skip to content

Fix/rdkit qed module clean#105

Open
jiaodu1307 wants to merge 2 commits into
K-Dense-AI:mainfrom
jiaodu1307:fix/rdkit-qed-module-clean
Open

Fix/rdkit qed module clean#105
jiaodu1307 wants to merge 2 commits into
K-Dense-AI:mainfrom
jiaodu1307:fix/rdkit-qed-module-clean

Conversation

@jiaodu1307
Copy link
Copy Markdown
Contributor

Use QED module for QED score calculation
ref: https://www.rdkit.org/docs/source/rdkit.Chem.QED.html#rdkit.Chem.QED.qed

Copy link
Copy Markdown

@Gonzih Gonzih left a comment

Choose a reason for hiding this comment

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

LGTM — correct fix with the right approach.

Descriptors.qed is a thin alias that internally delegates to rdkit.Chem.QED.qed, but the canonical RDKit API is QED.qed(mol). Using the module directly is clearer, more explicit, and consistent with how the rest of the codebase already imports and uses QED (the import line is updated correctly in the same diff to add QED to the existing import). Two-line change, no side effects.

Copy link
Copy Markdown

@Gonzih Gonzih left a comment

Choose a reason for hiding this comment

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

Correct fix. Descriptors.qed does not exist — QED lives in its own rdkit.Chem.QED module. Adding the explicit import and calling QED.qed(mol) is the right approach per the RDKit docs. Clean and minimal.

Note: this file is also modified by PR #97. Whichever merges second will need a rebase. Both fixes touch different lines so the combination is straightforward.

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.

2 participants