Skip to content
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

Add a positive test unit for _is_smiles when chem is none in test_compound_identifier.py #1332

Merged
merged 4 commits into from
Nov 18, 2024

Conversation

Ajoke23
Copy link
Contributor

@Ajoke23 Ajoke23 commented Oct 17, 2024

…_identifier.py

Thank you for taking your time to contribute to Ersilia, just a few checks before we proceed

  • Have you followed the guidelines in our Contribution Guide
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Description

Added a positive test for _is_smiles method when chem is none in the CompoundIdentifier class. The objective is to verify the
_is_smiles method in CompoundIdentifier class when Chem module is not available i.e when chem=None

Changes to be made

  • Patched _pubchem_smiles_to_inchikey which is used for converting SMILES to InCHiKey when chem is none

  • Mocked _pubchem_smiles_to_inchikey method to return "InchiKey" to simulate a valid InchiKey result. Therefore indicating SMILES input is valid

  • Test with a Valid SMILES input. In this code, I used the valid SMILES string used is: CCO which represent Ethanol is passed to is_smiles method.

Status

Ran the test locally using pytest and it's passed all check. Proof: is_smiles - positive.log

To do

Awaiting mentors feedback

Is this pull request related to any open issue? If yes, replace issueID below with the issue ID

Related to #1319

@DhanshreeA DhanshreeA merged commit ea74556 into ersilia-os:master Nov 18, 2024
18 checks passed
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