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 tests for _is_smiles, _is_inchikey, and guess_type methods in CompoundIdentifier class #1358

Merged
merged 8 commits into from
Nov 21, 2024

Conversation

musasizivictoria
Copy link
Contributor

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

This commit introduces unit tests for the following methods in the CompoundIdentifier class:

  1. _is_smiles(): Positive and negative cases, both when RDKit (Chem) is available and not available.
  2. _is_inchikey(): Valid and invalid InChIKey tests to ensure correct identification.
  3. guess_type(): Tests to validate various inputs, including:
  • InChIKeys
  • SMILES strings
  • Unprocessable inputs (like None and UNPROCESSABLE_INPUT)
  • Whitespace-only inputs (\n, \t, space)
  • Non-character inputs (numbers, floats, and random Unicode)

Changes to be made

Added the following tests:

  1. _is_smiles() positive and negative tests for RDKit presence and absence.
  2. _is_inchikey() positive and negative tests for valid/invalid keys.
  3. guess_type() tests covering:
  • Valid InChIKeys
  • Valid SMILES strings
  • Unprocessable inputs
  • Whitespace-only inputs
  • Non-character inputs

Status

  • Tests written and validated locally using pytest.
  • All tests passing successfully.
    Screenshot from 2024-10-28 00-53-17

To do

  • Documentation

Related to #1319

@musasizivictoria
Copy link
Contributor Author

musasizivictoria commented Oct 29, 2024

@DhanshreeA kindly this is ready for review. Thanks

cc @Malikbadmus

@DhanshreeA
Copy link
Member

Quite a number of good tests here, I need to resolve conflicts in the PR before merging.

@DhanshreeA DhanshreeA merged commit 0d8c248 into ersilia-os:master Nov 21, 2024
18 checks passed
@musasizivictoria musasizivictoria deleted the unittestsuite branch November 21, 2024 19:22
@musasizivictoria musasizivictoria restored the unittestsuite branch November 21, 2024 19:22
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