-
Notifications
You must be signed in to change notification settings - Fork 81
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
Reformat loaders for different smiles paths #1211
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1211 +/- ##
==========================================
+ Coverage 85.73% 85.89% +0.15%
==========================================
Files 53 53
Lines 4802 4799 -3
==========================================
+ Hits 4117 4122 +5
+ Misses 685 677 -8 ☔ View full report in Codecov by Sentry. |
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.
LGTM. Left a couple comments that should be quick changes. Thanks!
0d5cf5c
to
d7ea5e2
Compare
PR Summary:
Updating the loading paths to deal with issues loading long strings as smiles. Initial loading uses the Path package to check if the path exists, before checking to see if rdkit can load a valid smiles file. However, Path throws an error if the Path is too long:
This PR better handles this type of smiles, and also splits the handling of the rdkit and pybel backends for smiles loading, both from files and from strings. Discussion with @mattwthompson helped delineate the more specific loaders for these different methods.
PR Checklist