-
Notifications
You must be signed in to change notification settings - Fork 40
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
fix: use rdkit API to remove implicit hydrogens #2278
Conversation
Signed-off-by: Jinzhe Zeng <[email protected]>
Warning Rate limit exceeded@njzjz has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 9 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request primarily involve the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ConvertSMILES
participant RDKit
User->>ConvertSMILES: Request SMILES conversion
ConvertSMILES->>RDKit: Generate SMILES string
RDKit-->>ConvertSMILES: Return SMILES string
ConvertSMILES-->>User: Return final SMILES string
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Signed-off-by: Jinzhe Zeng <[email protected]>
CodSpeed Performance ReportMerging #2278 will not alter performanceComparing Summary
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
reacnetgenerator/_path.py (1)
Line range hint
1-239
: Approve removal of_re
method and suggest documentation updateThe removal of the
_re
method is consistent with the changes in theconvertSMILES
method and aligns with the PR objective to use the RDKit API directly for handling hydrogens. This simplification improves code maintainability and reduces potential sources of errors.Consider updating the class documentation to reflect the removal of the
_re
method and the changes in the SMILES conversion process, if applicable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- reacnetgenerator/_path.py (1 hunks)
- tests/test_reacnetgen.py (0 hunks)
💤 Files with no reviewable changes (1)
- tests/test_reacnetgen.py
🔇 Additional comments (2)
reacnetgenerator/_path.py (2)
234-237
: Approve changes to handle implicit hydrogens explicitlyThe modifications to the
convertSMILES
method align with the PR objective to remove implicit hydrogens using the RDKit API. By settingNoImplicit
to True for each atom and updating the property cache, the code ensures that all hydrogens are explicitly represented in the molecule. This change improves the accuracy of the SMILES representation and prevents potential issues with implicit hydrogen handling in downstream processes.
Line range hint
1-239
: Summary of changes and their impactThe modifications in this PR successfully implement the objective of using the RDKit API to remove implicit hydrogens. The changes primarily affect the SMILES conversion process in the
_CollectPaths
class:
- The
_re
method has been removed, simplifying the code.- The
convertSMILES
method now directly uses RDKit's functionality to handle hydrogens explicitly.These changes should improve the accuracy of SMILES representations and ensure consistency in hydrogen handling throughout the molecule processing pipeline. The code is now more streamlined and better aligned with RDKit's capabilities.
Signed-off-by: Jinzhe Zeng <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2278 +/- ##
==========================================
+ Coverage 94.98% 96.05% +1.06%
==========================================
Files 17 17
Lines 1415 1418 +3
==========================================
+ Hits 1344 1362 +18
+ Misses 71 56 -15 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores