Skip to content

Comments

feat: get smiles from pubchem#546

Merged
Kohulan merged 7 commits intodevelopmentfrom
dev-kohulan
Apr 3, 2025
Merged

feat: get smiles from pubchem#546
Kohulan merged 7 commits intodevelopmentfrom
dev-kohulan

Conversation

@Kohulan
Copy link
Member

@Kohulan Kohulan commented Apr 3, 2025

No description provided.

@Kohulan Kohulan requested a review from sriramkanakam87 April 3, 2025 09:15
@Kohulan Kohulan self-assigned this Apr 3, 2025
@Kohulan Kohulan added the enhancement New feature or request label Apr 3, 2025
@codecov
Copy link

codecov bot commented Apr 3, 2025

Codecov Report

Attention: Patch coverage is 94.98607% with 18 lines in your changes missing coverage. Please review.

Project coverage is 92.00%. Comparing base (2cbd16c) to head (ec40b6c).
Report is 8 commits behind head on development.

Files with missing lines Patch % Lines
app/routers/chem.py 21.73% 18 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           development     #546      +/-   ##
===============================================
+ Coverage        91.61%   92.00%   +0.39%     
===============================================
  Files               47       50       +3     
  Lines             2792     3141     +349     
===============================================
+ Hits              2558     2890     +332     
- Misses             234      251      +17     
Flag Coverage Δ
service 92.00% <94.98%> (+0.39%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Kohulan Kohulan requested a review from Copilot April 3, 2025 10:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new feature for retrieving canonical SMILES from PubChem based on a flexible chemical identifier input. Key changes include the addition of comprehensive tests for the PubChemClient, updates to the FastAPI router to incorporate a new endpoint, and modifications to schema definitions and utility functions to support PubChem retrieval.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_pubchem_client.py New test suite for various PubChem retrieval scenarios
tests/test_chem.py Update to test parameters for chemical input validation
app/schemas/pubchem_schema.py New Pydantic model for PubChem response
app/schemas/coconut.py Minor formatting adjustments in field names
app/routers/chem.py Addition of a new endpoint to retrieve SMILES from PubChem
app/modules/toolkits/cdk_wrapper.py Format adjustments in the Murcko framework helper function
app/modules/pubchem_retrieve.py Implementation of PubChemClient with multiple query methods
app/modules/coconut/preprocess.py Update to use consistent naming for Murcko framework fields
app/modules/coconut/descriptors.py Consistent use of Murcko framework retrieval in descriptors
app/main.py Metadata update and OpenAPI tag restructuring
CHANGELOG.md Changelog updated to reflect the new feature changes
Comments suppressed due to low confidence (2)

app/modules/pubchem_retrieve.py:250

  • The SMILES detection logic uses a length cutoff of 10 characters, which might inadvertently exclude valid SMILES strings. Consider revising the condition or adding additional checks to better differentiate SMILES inputs.
and len(user_input) <= 10

app/routers/chem.py:1226

  • The logic for detecting InChIKey identifiers is brittle due to explicit index and format checks. Consider using a regular expression matching the standard InChIKey format for improved robustness.
elif (len(identifier) == 27 and identifier.count("-") == 2 and all(c.isupper() or c.isdigit() or c == "-" for c in identifier) and identifier[14] == "-" and identifier[25] == "-"):

Kohulan and others added 3 commits April 3, 2025 12:37
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Kohulan Kohulan merged commit c2e3efa into development Apr 3, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Get SMILES from name using Pubchem fix the murcko framework spelling

2 participants