Closes #151

[leoisl]

Closes #151

Switching to uint16 for coverage information reduced the RAM usage for pandora compare on the full klebs dataset from 17.1 GB to 13.5 GB.

This dataset contains only 151 samples, and the more samples, the larger is the reduction, so this change is important for scalability.

[mbhall88]

A couple of variable name suggestions.

As a more general statement. I would like if we could move away from having these enormous tests that test a huge number of different states and conditions. A test should test one single case and it should be fairly easy to see what the test is testing from reading the code.
The test here that you have added realistically should be broken up into many smaller tests. Additionally, the name of a test should tell the reader what the test does and what the expected outcome is. See this test suite for an example of testing a single function.

[leoisl]

As a more general statement. I would like if we could move away from having these enormous tests that test a huge number of different states and conditions. A test should test one single case and it should be fairly easy to see what the test is testing from reading the code.
The test here that you have added realistically should be broken up into many smaller tests. Additionally, the name of a test should tell the reader what the test does and what the expected outcome is. See this test suite for an example of testing a single function.

Indeed, you are right, it is a better approach. I will break this single test into 6 different tests.

[leoisl]

All your suggestions were done in 910e49b, please check if further improvements should be made.

[rmcolq]

I'm happy to merge/rebase this in now

…

-------- Original message -------- From: leoisl <[email protected]> Date: 02/07/2019 23:22 (GMT+00:00) To: rmcolq/pandora <[email protected]> Cc: Rachel Norris <[email protected]>, Review requested <[email protected]> Subject: Re: [rmcolq/pandora] Closes #151 (#152) All your suggestions were done in new commit, please check if further improvements should be made. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub<#152>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ACLIWOZBOYC2E2V5T6MF4P3P5PIL7ANCNFSM4H4GAPJQ>.

[mbhall88]

The tests look good to me now.