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 bio array tests #101

Merged
merged 9 commits into from
Jul 16, 2020
Merged

Conversation

graemevissers
Copy link
Contributor

Add unit tests to BioArray testList in BioCollectionsTests module

Changes

Add testCases:

  • isEqual
  • toString
  • toMonoIsotopicMass
  • toAverageMass
  • toMonoIsotopicMassWith
  • toAverageMassWith
  • toCompositionVector

to testList BioArray in BioCollectionsTests module.

Description

This contribution adds seven unit tests to the BioCollectionsTests module, and I plan on adding similar tests to other testLists in that module. These unit tests all seem to be passing, but this is my first contribution to BioFSharp, so please let me know if there are ways to improve my contributions.

[Required] please make sure you checked that

  • The project builds without problems on your machine

Copy link
Member

@kMutagene kMutagene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @graemevissers ,

Thank you for your contributions. everything looks fine. I just have a single comment on your last test. It would be awesome if you could further isolate that test (see my comment). Otherwise, cool stuff. We can get this merged soon.

)

testCase "toCompositionVector" (fun () ->
let testCompVec = Array.zeroCreate 26
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this test you are also dependent on BioItem.symbol working as expected. It would make the test more isolated if you initialize the test composition vector without being dependent on other code (e.g. using the correct characters in lines 182-184).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add changes to this PR, the only thing you have to do is add new commits to your branch, they will be automatically added to the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kMutagene,

Thanks for the suggestions. I updated the code to no longer be dependent on BioItem.symbol and committed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@kMutagene kMutagene self-assigned this Jul 16, 2020
Expect.floatClose
Accuracy.medium // High accuracy was not passing test
(testProt |> BioArray.toAverageMass)
// Masses obtained from University of Washington Proteomics Resource https://proteomicsresource.washington.edu/protocols06/masses.php
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also like that you included sources here, so when we have to revisit these tests we have the references in the code.

@kMutagene kMutagene merged commit 00f9b39 into CSBiology:developer Jul 16, 2020
@kMutagene
Copy link
Member

Referencing #30 to track progress

graemevissers added a commit to graemevissers/BioFSharp that referenced this pull request Jul 17, 2020
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