Skip to content

Conversation

nvaulin
Copy link
Member

@nvaulin nvaulin commented Feb 26, 2024

Review MRPS7

Comment on lines +88 to +89
return sum(amino_acid_weights.get(aa, 0) for aa in self._sequence)

Copy link

Choose a reason for hiding this comment

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

Код не учитывает потерю воды при образовании аминокислотной цепи. Например,

        water_mw = 18
        for aa in list_input_seq:
            total_mw = sum(aa_weight_dict[a] for a in list_input_seq)
            mw_water_removed = (total_mw - (water_mw * (len(list_input_seq)-1)))
        return mw_water_removed

Copy link
Member Author

Choose a reason for hiding this comment

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

Отличное замечание. Я бы его еще оформил с подсветкой синтаксиса:
image

Comment on lines +45 to +46
gc_count = sum(base in {'G', 'C'} for base in self._sequence)
return gc_count / len(self._sequence)
Copy link

Choose a reason for hiding this comment

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

Поскольку это абстрактный класс, то методов здесь не должно быть

raise ValueError("Invalid sequence for the given type.")
self._sequence = sequence

@abstractmethod
Copy link

Choose a reason for hiding this comment

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

Насколько я понимаю, декоратор @abstract должен быть перед def init

Copy link
Member Author

Choose a reason for hiding this comment

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

Да, тут вообще все методы должны быть абстрактными

Comment on lines +76 to +78
valid_amino_acids = {'A', 'R', 'N', 'D', 'C', 'Q', 'E', 'G', 'H', 'I',
'L', 'K', 'M', 'F', 'P', 'S', 'T', 'W', 'Y', 'V'}
return all(amino_acid in valid_amino_acids for amino_acid in sequence)
Copy link

Choose a reason for hiding this comment

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

Можно было добавить ситуацию когда подаются не заглавные буквы, например c помощью set:

    unique_chars = set(seq)
    single_letter = set('GALMFWKQESPVICYHRNDTgalmfwkqespvicyhrndt')

Comment on lines +51 to +52
def is_valid(self, sequence: str) -> bool:
return all(nucleotide in {'A', 'C', 'G', 'T'} for nucleotide in sequence)
Copy link

Choose a reason for hiding this comment

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

То же самое про заглавные и строчные буквы, можно добавить такую проверку:

        nucleotides_dna = set('ATGCatgc')
        nucleotides_rna = set('AUGCaugc')

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