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

added test for alignment_info.py #227

Merged
merged 1 commit into from
Aug 26, 2024
Merged

added test for alignment_info.py #227

merged 1 commit into from
Aug 26, 2024

Conversation

cat-bug
Copy link
Collaborator

@cat-bug cat-bug commented Aug 22, 2024

also removed unused code, change the order of expected/actual in number of asserts

Copy link
Collaborator

@andrewprzh andrewprzh left a comment

Choose a reason for hiding this comment

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

Looks good! Just a couple minor changes

@@ -10,6 +10,8 @@
from .common import get_read_blocks
from .polya_verification import shift_polya, shift_polyt

REGION_TO_CHECK_LEN = 12
Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree with moving to constants, but I'd also put it back into AlignmentInfo as a global field (AlignmentInfo.REGION_TO_CHECK_LEN)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

src/common.py Outdated

@classmethod
def get_match_events(cls):
return [cls.match, cls.seq_match, cls.seq_mismatch]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think using set might be faster as we use for in checks.
I don't really know what is faster for small collections

Copy link
Collaborator

Choose a reason for hiding this comment

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

Simple tests show that even for collections of size 2 checking element in set works a bit faster than in list, from size 3 difference is noticeable
So, yeah, let's switch to set, idk why I used lists before :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

src/common.py Outdated

@classmethod
def get_ins_del_match_events(cls):
return [cls.match, cls.insertion, cls.deletion, cls.seq_match, cls.seq_mismatch]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think using set will be faster as we use for in checks

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@andrewprzh andrewprzh merged commit aa62894 into master Aug 26, 2024
1 check 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