Add more CMS provider for SMILES #932
Conversation
Codecov ReportPatch coverage is
📢 Thoughts on this report? Let us know!. |
| @@ -91098,7 +91098,7 @@ | |||
| "description": "Convert SMILES to various molecular formats using different toolkits.", | |||
| "homepage": "https://api.naturalproducts.net/latest/docs#/convert/SMILES_convert_to_Formats_convert_formats_get", | |||
| "name": "Convert SMILES to various molecular formats via the Cheminformatics Microservice", | |||
| "uri_format": "https://api.naturalproducts.net/latest/convert/formats?smiles=$1&toolkit=cdk" | |||
| "uri_format": "https://api.naturalproducts.net/latest/convert/formats?smiles=$1&toolkit=$2" | |||
There was a problem hiding this comment.
The Bioregistry doesn't currently support this kind of syntax with multiple placeholders, but this is something we could consider in the future if you make an issue about it
There was a problem hiding this comment.
I formalized this curation guideline in a unit test in 41ef3b1 so we are reminded by CI to address this before finishing the PR
| "description": "Generate SMILES from a given IUPAC name or a SELFIES representation.", | ||
| "homepage": "https://api.naturalproducts.net/latest/docs#/convert/IUPACname_or_SELFIES_to_SMILES_convert_smiles_get", | ||
| "name": "Generate SMILES via the Cheminformatics Microservice", | ||
| "uri_format": "https://api.naturalproducts.net/latest/convert/smiles?input_text=$1&representation=iupac" |
There was a problem hiding this comment.
this doesn't make sense since the $1 doesn't represent a SMILES string but rather free text. please delete this one
| "uri_format": "https://api.naturalproducts.net/latest/convert/mol2D?smiles=$1&toolkit=$2" | ||
| }, | ||
| { | ||
| "code": "cms.convertMol3D", |
There was a problem hiding this comment.
we need to keep all codes lowercase. maybe we can use snake_case to separate words instead. Also, convert doesn't imply direction, so maybe cms.to_mol3d or even cms.mol3d would be better
better to put as little information in the URI as possible that can be passed separately as parameters
|
Thanks @cthoyt for your review. |
|
@NishaSharma14 nope, just wanted you to review my changes before merging |
Looks good to me. |
No description provided.