Skip to content

fix(rdkit): replace FractionCsp3 with Lipinski.FractionCSP3#97

Open
jiaodu1307 wants to merge 2 commits into
K-Dense-AI:mainfrom
jiaodu1307:fix/rdkit-fraction-csp3
Open

fix(rdkit): replace FractionCsp3 with Lipinski.FractionCSP3#97
jiaodu1307 wants to merge 2 commits into
K-Dense-AI:mainfrom
jiaodu1307:fix/rdkit-fraction-csp3

Conversation

@jiaodu1307

Copy link
Copy Markdown
Contributor

fix(rdkit): replace FractionCsp3 with Lipinski.FractionCSP3

@Gonzih Gonzih left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM — correct fix.

Descriptors.FractionCsp3 is a deprecated/aliased accessor; the canonical function is Lipinski.FractionCSP3, which is what the RDKit documentation specifies. The one-line change is accurate and minimal. The Lipinski module is already imported in the file, so no additional import is needed.

@Gonzih Gonzih left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Correct fix. FractionCSP3 lives in the Lipinski module, not Descriptors — the old call would raise AttributeError at runtime. Lipinski is already imported so this is a clean one-line patch. LGTM.

Note: this file is also touched by PR #105. Whichever merges second will need a rebase to avoid a conflict.

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