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 masking chains by CDRs, fix naming of runction chain masking #3

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

egor4k
Copy link

@egor4k egor4k commented Oct 12, 2023

No description provided.

@@ -36,13 +39,41 @@ def get_index_of_masks(len_seq:int,max_parts:int,max_length:int)->list[int]:
if s>=t[0] and s<=t[1] or e>=t[0] and e<=t[1]:
f=True
break
if not f:
if not f and (s==0 or not mask[s-1]) and (e==len_seq-1 or not mask[e+1]):
Copy link

Choose a reason for hiding this comment

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

Очень сложно понять, что в этой функции происходит, можно однобуквенные переменные заменить на что-то более содержательное? под сложные условия тоже хорошо заводить булевы переменные, если это позволяет лучше понять логику

tuples.append((s,e))
for i in range(s,e+1):
mask[i]=1
return mask

def featurize(batch, device,max_parts=6,max_length=6):
def get_mask_cdrs_one_chain(chain,max_parts,max_length,indexes_of_cdrs):
Copy link

Choose a reason for hiding this comment

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

Если в этой функции цель - получить бинарную маску, где нули стоят на месте нужного региона, то можно намного проще: создать массив из нулей, получить положение нужного региона, заменить на месте 0 на 1 через оператор среза, без циклов.

@norsage
Copy link

norsage commented Dec 8, 2023

Пожелание по всему коду: пройтись flake8, проставить аннотации типов, избавиться от однобуквенных переменных, если они живут дольше, чем в одной строчке.

@egor4k egor4k requested a review from norsage December 13, 2023 13:37
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