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

Merged
merged 2 commits into from
Nov 18, 2024

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 negative test for the _is_inchikey method

Changes to be made

  • Add negative test case for _is_inchikey

  • Test the following invalid InChIKeys:
    No hyphens: BSYNRYMUTXBXSQUHFFFAOYSA
    Incorrect length: BSYNRYMUTXBXSQ-UHFFFAOYSA-XY
    Numerical characters: 12345678901234-1234567890-X
    Underscore instead of hyphens: BSYNRYMUTXBXSQ_UHFFFAOYSA-N
    No last part: BSYNRYMUTXBXSQ-UHFFFAOYSA

Status

  • I added the negative 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

@DhanshreeA
Copy link
Member

Hey @adebisi4145 could you address the conflicts in this PR?

@DhanshreeA DhanshreeA merged commit c4c790a 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