Review RASA2 #28
Review RASA2 #28
Conversation
| print(str(self.sequence)) | ||
|
|
||
| def is_valid_alphabet(self): | ||
| valid_nucleotides = {'A', 'C', 'G', 'T', 'U'} |
There was a problem hiding this comment.
Можно было добавить проверку для строчных букв, либо перевести все в заглавные
| if nucl.upper() == 'A': | ||
| transcribe_sequence += nucl | ||
| elif nucl.upper() == 'G': | ||
| transcribe_sequence += nucl | ||
| elif nucl.upper() == 'C': | ||
| transcribe_sequence += nucl |
There was a problem hiding this comment.
Как вариант, можно было бы это не писать и только рассматривать случай с T:
def transcribe(self):
list_input = list(self.seq)
for i in range(len(self.seq)):
if (list_input[i] == 'T'):
list_input[i] = 'U'
elif (list_input[i] == 't'):
list_input[i]='u'
return "".join(list_input)Или еще короче:
def transcribe(self) -> RNASequence:
return RNASequence(str(self).translate(str.maketrans(('Tt'), ('Uu'))))There was a problem hiding this comment.
О, вот, с подсветкой и само предложение тоже супер
| transcribe_sequence += nucl | ||
| elif nucl.upper() == 'C': | ||
| transcribe_sequence += nucl | ||
| elif nucl.upper() == 'T': |
There was a problem hiding this comment.
Не очень понимаю, если выполнен метод upper(), почему на следующей строчке рассмотрен вариант t строчной?
|
|
||
|
|
||
| class BiologicalSequence: | ||
| @abstractmethod |
There was a problem hiding this comment.
абстрактный метод задан верно, только можно добавить наследование:
class BiologicalSequence(ABC):
| quality_threshold: int = 0, output_filename='') -> None: | ||
| seqs = SeqIO.parse(input_path, 'fastq') | ||
| if seqs is None: | ||
| raise ValueError('Your fastq_files are None') |
There was a problem hiding this comment.
тут не совсем понятный вывод получается. файла нет или файл пустой, лучше поменять или вывести потенциальные ошибки
| if not os.path.exists(output_dir): | ||
| os.mkdir(output_dir) | ||
|
|
||
| with open(os.path.join(output_dir, f'{output_filename}.fastq'), 'w') as fq: |
There was a problem hiding this comment.
хорошая конструкция, для того, что бы не забывать закрывать файл, но чуть проще читается код и чувствуется больше уверенености, когда в конце есть явное закрытие файла
| length_bounds[1]: | ||
| gc_content = SeqUtils.GC123(record.seq) | ||
| if gc_bounds[1] >= gc_content[0] >= gc_bounds[0]: | ||
| SeqIO.write(record, fq, 'fastq') |
There was a problem hiding this comment.
| SeqIO.write(record, fq, 'fastq') | |
| SeqIO.write(record, fq, 'fastq') | |
| fq.close() |
| SeqIO.write(record, fq, 'fastq') | ||
|
|
||
|
|
||
| class BiologicalSequence: |
| print(str(self.sequence)) | ||
|
|
||
| def is_valid_alphabet(self): | ||
| valid_nucleotides = {'A', 'C', 'G', 'T', 'U'} |
There was a problem hiding this comment.
хорошая проверка нуклеиновых кислот, но они могут быть прописаны в другом регистре, тогда стоит добавить с нижним регистром тоже
| transcribe_sequence += nucl | ||
| elif nucl.upper() == 'T': | ||
| if nucl == 't': | ||
| transcribe_sequence += 'u' |
There was a problem hiding this comment.
Это вероятно лишнее, потому что все в верхний регистр переводится же
| 3)] | ||
| for divided_acid in divided_acids: | ||
| if divided_acid not in self.AMINOACIDS_DICT.keys(): | ||
| raise ValueError('Non-protein aminoacids in sequence') |
There was a problem hiding this comment.
тут стоит добавить какое-то объяснение для пользователей, вероятно пользователю будет не сразу понятна ошибка. Наличие проверки - хорошо, но её как будто бы лучше вынести перед работой или в самом начале функции
| for letter in self.sequence: | ||
| if letter not in self.TRANSCRIBE_DICT.keys(): | ||
| return False | ||
| return True |
There was a problem hiding this comment.
Монжо чуть чуть изменить эту функцию, так она будет работать побыстрее
| return True | |
| valid_alphabet_set = set(self.TRANSCRIBE_DICT.keys()) | |
| return set(self.sequence).issubset(valid_alphabet_set) |
There was a problem hiding this comment.
Отличное замечание. Даже тут не только быстрее, но и просто более питонистый стиль. Не вручную копаем отдельные элементы коллекции, а используем более высокоуровневые методы самих коллекций.
| Parameters: | ||
| *parameters (str): Variable number of DNA or RNA sequences and the tool name. | ||
|
|
||
| Returns: |
There was a problem hiding this comment.
Это прям сильно. слишком подробно, в коде наверное стоит писать более емко все
| elif answer is None: | ||
| raise ValueError('Answer is None') | ||
| else: | ||
| return answer |
There was a problem hiding this comment.
Хорошая работа, она логичная и структурно выдержанная, чувствуется, что писалась самостоятельно, докстринга шикарно, но слишком развернутая, стоит чуть более емко писать, код воспринимается понятно и легко, осень хороший код))
Review RASA2