tacitvenom / genomics_algo

MIT License
1 stars 1 forks source link

Contribution/issue21 #37

Closed SvoONs closed 3 years ago

SvoONs commented 3 years ago

The current implementation is tested quite thoroughly contentwise but performance is rather poor at the moment for high values of k and d. I added a test that runs ~2 mins, if not skipped. I think main reason for the poor performance is line 94 in misc_algos.py, the implementation greedily computes all possible patterns with length k and afterwards filters the ones with at most d mismatches

Fixes #21

SvoONs commented 3 years ago

I have a failing doc test because of the order of items in the result. Will check how to solve this

SvoONs commented 3 years ago

Adapted according to your comments, I can rebase and squash commits before merge. Let me know what you think and if I should address something else

tacitvenom commented 3 years ago

Adapted according to your comments, I can rebase and squash commits before merge. Let me know what you think and if I should address something else

Looks great. I'll later add tests for validate function with some other PR. It wasn't related to the issue you addressed anyway. Thanks again!

SvoONs commented 3 years ago

Adapted according to your comments, I can rebase and squash commits before merge. Let me know what you think and if I should address something else

Looks great. I'll later add tests for validate function with some other PR. It wasn't related to the issue you addressed anyway. Thanks again!

Only you can merge it, I think

tacitvenom commented 3 years ago

Adapted according to your comments, I can rebase and squash commits before merge. Let me know what you think and if I should address something else

Looks great. I'll later add tests for validate function with some other PR. It wasn't related to the issue you addressed anyway. Thanks again!

Only you can merge it, I think

aah, right, I'll merge this one and see how to add you too.