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

Local random object and Skipping malformed denovo variants #61

Merged
merged 4 commits into from
Jul 18, 2023

Conversation

leoisl
Copy link
Collaborator

@leoisl leoisl commented Jul 17, 2023

This PR fixes two small issues:

  1. Fixes a bug introduced in the previous PR, by using a local random object instead of the global random module. In Python, the random module is not process-safe or thread-safe by default. It uses a global instance of the pseudorandom number generator, which is shared between all uses of the random module in the same Python process. It's not designed to handle the concurrent usage scenario well, which is what we have in make_prg. Basically, whenever we need to get_majority_consensus_from_MSA(), we set the seed of the random object WRT the sequences of the alignment, so that we get the exact same consensus given the same alignment. But we process multiple MSAs at the same time, which could cause issue, e.g. setting random.seed() of one MSA while generating the consensus of another MSA (I was having issues with integration tests not being reproducible). We now use a local random object that prevents this issue;
  2. We now skip denovo variants that for some very rare reason might have N in it (closes Ignore Ns that might have slipped through when updating PRGs #58);

Most of the changed files in this PR is updating the zip files in integration tests

@leoisl
Copy link
Collaborator Author

leoisl commented Jul 17, 2023

CI is failing trying to install scikit-learn for some obscure reason I could not fix yet...

Tests run and pass on my end, total coverage is 99.74%, so it has not decreased:

Required test coverage of 98% reached. Total coverage: 99.74%
=========================================================================================== 494 passed, 1 skipped, 4 warnings in 58.95s ===========================================================================================

Copy link
Contributor

@iqbal-lab iqbal-lab left a comment

Choose a reason for hiding this comment

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

Approved!

@leoisl leoisl merged commit 3626d01 into iqbal-lab-org:dev Jul 18, 2023
0 of 4 checks passed
@leoisl leoisl deleted the v0.5.0 branch July 19, 2023 15:04
@leoisl leoisl mentioned this pull request Jul 20, 2023
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.

3 participants