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 positive test for the _is_inchikey method in CompoundIdentifier class #1333

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adebisi4145
Copy link
Contributor

@adebisi4145 adebisi4145 commented Oct 18, 2024

…lass

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

The _is_inchikey method in CompoundIdentifier class checks whether a given string is a valid InChIKey. This method checks whether the string provided is exactly 27 characters long, splits the string into 3 parts using hyphen (-) as the seperator and then ensure that the first part has 14 characters, the second part has 10 characters, and the third part has 1 character.

This PR adds positive test for the _is_inchikey method

Changes to be made

  • Add positive test case for _is_inchikey

  • Test the following valid 27-character InChIKeys:
    Aspirin: BSYNRYMUTXBXSQ-UHFFFAOYSA-N
    Caffeine: BQJCRHHNABKAKU-KBQPJGBKSA-N
    Ibuprofen: ZJPODVODJYKFHM-UHFFFAOYSA-N

Status

  • I added the positive test case for _is_inchikey
  • I ran all tests locally here

To do

If this is a work in progress, Replace this line with your next steps

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

Related to #1319

@adebisi4145
Copy link
Contributor Author

Hello @DhanshreeA , @Malikbadmus I just discovered that the _is_inchikey method currently does not enforce case sensitivity in its validation

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.

1 participant