Conversation
ailiskab-hub
left a comment
ailiskab-hub
left a comment
There was a problem hiding this comment.
Ну главное, что вы сделали задание)
| -output (dict[str,str]) - filtered dictionary, which consists of entities that fulfill entered parameters. | ||
| """ | ||
|
|
||
| def average_quality(record): |
There was a problem hiding this comment.
Лучше отпределять функцию вне функции
| def __init__(self, sequence): | ||
| self.sequence = sequence | ||
|
|
||
| @abstractmethod |
| sequence_type: str = None # Это будет переопределено в подклассах | ||
|
|
||
| def __init__(self, sequence): | ||
| self.sequence = sequence |
There was a problem hiding this comment.
Тут необходимо добавить структуру с super(), чтобы наследование проходило корректно
| return ''.join(COMPLEMENT_MAP[base] for base in self.sequence) | ||
|
|
||
| def check_alphabet(self): | ||
| valid_bases = set('ATGCU') if self.sequence_type == 'DNA' else set('AUGC') |
There was a problem hiding this comment.
Можно ввести переменные, чтобы не создавать множество кадый раз
|
|
||
|
|
||
| class DNASequence(NucleicAcidSequence): | ||
| sequence_type = 'DNA' |
There was a problem hiding this comment.
Если sequence_type это атрибут модели, то нужно было использовать self.sequence_type
И снова не хватает super()
| return (self.complement()).replace('T', 'U') | ||
|
|
||
|
|
||
| class RNASequence(NucleicAcidSequence): |
| return self.sequence[index] | ||
|
|
||
| def __str__(self): | ||
| return self.sequence |
There was a problem hiding this comment.
Очень много дублирующегося кода в ДНК и РНК, почему бы не реализовать это в родительском классе?
| 'E': 147.131, 'Q': 146.146, 'G': 75.067, 'H': 155.156, 'I': 131.175, | ||
| 'L': 131.175, 'K': 146.189, 'M': 149.208, 'F': 165.192, 'P': 115.132, | ||
| 'S': 105.093, 'T': 119.119, 'W': 204.228, 'Y': 181.191, 'V': 117.148} | ||
| return sum(MOLECULAR_MASS_MAP[aa] for aa in self.sequence) |
There was a problem hiding this comment.
Тут тоже много дублирующегося кода, некоторые из методов можно было реализовать в BiologicalSequence, чтобы избегать повторения
stegodasha
left a comment
stegodasha
left a comment
There was a problem hiding this comment.
Вообще в целом ты большой/ая молодец))
| if len(seq) >= length_bounds[0] and len(seq) <= length_bounds[1] \ | ||
| and average_quality(seq) >= quality_threshold \ | ||
| and GC(seq.seq) >= gc_bounds[0] and GC(seq.seq) <= gc_bounds[1]: |
There was a problem hiding this comment.
Я бы постаралась вычислить GC состав один раз:
| if len(seq) >= length_bounds[0] and len(seq) <= length_bounds[1] \ | |
| and average_quality(seq) >= quality_threshold \ | |
| and GC(seq.seq) >= gc_bounds[0] and GC(seq.seq) <= gc_bounds[1]: | |
| gc_content = GC(seq.seq) | |
| if len(seq) >= length_bounds[0] and len(seq) <= length_bounds[1] \ | |
| and average_quality(seq) >= quality_threshold \ | |
| and gc_bounds[0] <= gc_content <= gc_bounds[1]: |
| COMPLEMENT_MAP = {'A': 'T', 'T': 'A', 'C': 'G', 'G': 'C', 'U': 'A'} if self.sequence_type == 'DNA' else { | ||
| 'A': 'U', 'U': 'A', 'C': 'G', 'G': 'C'} |
There was a problem hiding this comment.
Если COMPLEMENT_MAP предполагается быть константой, лучше выделить его как атрибут класса или объявить его вне метода.
There was a problem hiding this comment.
Ага, и когда он будет классовым атрибутом - то его уже не надо делать капсом
| def transcribe(self): | ||
| return (self.complement()).replace('T', 'U') |
There was a problem hiding this comment.
Я бы предложила использовать super() для вызова метода complement из родительского класса, чтобы убедиться, что будет использована правильная версия метода: return super().complement().replace('T', 'U')
| def __len__(self): | ||
| return len(self.sequence) | ||
|
|
||
| def __getitem__(self, index): | ||
| return self.sequence[index] | ||
|
|
||
| def __str__(self): | ||
| return self.sequence |
There was a problem hiding this comment.
Согласна с Алисой, я бы вынесла эти методы в родительский класс, чтобы их не дублировать
| MOLECULAR_MASS_MAP = {'A': 89.094, 'R': 174.203, 'N': 132.119, 'D': 133.104, 'C': 121.154, | ||
| 'E': 147.131, 'Q': 146.146, 'G': 75.067, 'H': 155.156, 'I': 131.175, | ||
| 'L': 131.175, 'K': 146.189, 'M': 149.208, 'F': 165.192, 'P': 115.132, | ||
| 'S': 105.093, 'T': 119.119, 'W': 204.228, 'Y': 181.191, 'V': 117.148} |
There was a problem hiding this comment.
Я бы вынесла MOLECULAR_MASS_MAP отдельно как атрибут класса.
3 participants
Review EVI5L