Conversation
Add function calculate_length and dict codon_table
With functions is_protein, run_protein_tools, molecular_weight
Add translation, mutations and level_of_hydrophobic
Revise translation, check_mutation and compute_hydrophobicity Add ValueError in translation and check_mutations and docstrings in translation, check_mutations and compute_hydrophobicity
Add raise in check_mutations and change return in compute_hydrophobicity
Add functions protein_to_dna, count_amino_acids
nvaulin
left a comment
nvaulin
left a comment
There was a problem hiding this comment.
Привет:)
❗️ Напоминание напомнить всем членам команды посмотреть фидбек.
Ниже можете найти подробные комментарии по коду. Тут некоторые общие моменты:
- Очень классный README.
- Небольшая проблемка со структурой репозитория. Ваш README тоже должен был бы лежать в папке)
- Операции над белками. Здорово что сделали каждый по 3. Это чуть больше, чем нужно было на оценку, но мы же тут не за оценкой а за знаниями и навыками. Некоторые комментарии по коду
- Не нашел вообще проблем с общей логикой за исключением каких-то пары моментов, которые отметил в коде.
- Кое-где формат вывода не очень хороший, обратите внимания
- Где-то высказал свои предложения по более питонистым решениям и неймингам.
- Хорошая история коммитов, хорошие сообщения. По коммитам прям видно как вы постепенно улучшали код. Тем не менее есть несколько коммитов, где за раз добавлено многовато). Лучше не кидать несколько функций в один коммит.
- Шо у вас CaptnClementine делает в pull-request'e?)
- По форматам выводов важное замечание. Лучше было бы возвращать в ваем случае словрь (ключ - белок в запросе, значение - результат), чем лист из тюплов. В мутациях мб стоило возвращать только список, и печатать какое то сообщение. Чтобы потом с этим списком можно было бы работать.
- Константы в начале кода - капсом!
Баллы
- README 2.7/2.5
- Операции работают, 5*1.5 = 7.5/7.5
- За формат вывода -0.3
- За качество кода -0.3 (кое-где нейминги и в начале прям по букве на строке - это сильно))
Итого: 9.6
Очень классная работа! Спасибо!
| @@ -0,0 +1,336 @@ | |||
| alphabet_protein = { | |||
There was a problem hiding this comment.
Круто что вы вынесли эти переменные отдельно вначале вне функций. Несколько моментов:
- Такие вещи в питоне называются "константами" и их принято называть целиком капсом
| alphabet_protein = { | |
| ALPHABET_PROTEIN = { |
-
Тут название понятное, но кажется можно придумать что-то получше. Что-то типа
proteinogenic_aminoacids. -
На надо тут было разбирать каждую букву на отдальной строке... Понятно, что не все в одну строку, но по несколько АК на строке
Аналогично про все константы ниже
|
|
||
| def is_protein(seq: str): | ||
| """ | ||
| Check the existence of a protein sequence, return boolean. |
There was a problem hiding this comment.
Ну оно не совсем existence проверяет)
|
|
||
| def is_rna(seq: str): | ||
| """ | ||
| Check the existence of a RNA sequence, return boolean. |
| def compute_molecular_weight(protein: str) -> tuple: | ||
| """ | ||
| Compute molecular weight (g/mol) of protein sequence. | ||
|
|
||
| Argument: | ||
| - protein (str): protein sequence. | ||
|
|
||
| Return: | ||
| - tuple with protein sequence and computed molecular | ||
| weight (float rounded to 3 decimal places). | ||
| """ | ||
| molecular_weight = 0 | ||
| for amino_acid in protein.upper(): | ||
| molecular_weight += amino_acid_masses[amino_acid] | ||
| return protein, round(molecular_weight, 3) |
There was a problem hiding this comment.
Хорошая докстринга, здорово что упомянули размерность.
Разве что формат вывода у вашей функции немного странный. Я запускаю функцию типа compute_molecular_weight('MTNPQ) и я ожидаю получить число. А получаю кортеж.
Если бы у вас было много белков на входе, можно было бы выдать таблицу соотвествия белок->масса. Это надо сделать с помощью словаря (значение - белок). Кортеж тут не лучшее решение. По сути вы просто возвращаете белок который пользователь вам и дал)
| def compute_length(protein: str) -> tuple: | ||
| """ | ||
| Compute the length of the input protein sequence. | ||
|
|
||
| Argument: | ||
| - protein (str): protein sequence. | ||
|
|
||
| Return: | ||
| - tuple with protein sequence and computed length. | ||
| """ | ||
| return protein, len(protein) |
There was a problem hiding this comment.
Та же история про возвращение кортежа.
С подсчетом длины хитро:) Не поспоришь.
Это было бы на самом деле даже немного оправдано, если бы я мог давать аминокислоты в трехбуквенном коде
| if protein[-1] != "*": | ||
| raise ValueError("Stop (*) is absent") | ||
| if protein[0] != "M": | ||
| raise ValueError("Start (M) is absent") |
There was a problem hiding this comment.
У вас же есть аналогичные ошибки в translate_rna. Мб их не нужно дублировать.
|
|
||
| Examples: | ||
| - "AUGGUAGGGAAAUUUUGA", "MVGKF*" -> "Protein without mutations." | ||
| - "AUGGUAGGGAAAUUUUGA", "MGGVF*" -> "Mutations:G2, V4." |
There was a problem hiding this comment.
Круто что вы показываете что заменилось! Тут было бы использовать все таки нотацию типа "K2N" где K - то что должно быть по РНК, и N - то что по факту
| contain only three arguments: RNA sequence, protein sequence | ||
| and the name of procedure itself. | ||
| """ | ||
| *seqs, procedure = args |
There was a problem hiding this comment.
Это ок, как в прошлом ДЗ, но все таки в реальных тулах такое может быть не очень очевидно. Я бы сделал название процедуры именованным аргументом.
| """ | ||
| *seqs, procedure = args | ||
| results = [] | ||
| d_of_functions = { |
There was a problem hiding this comment.
| d_of_functions = { | |
| functions = { |
|
|
||
| **protein_tools.py** - is a tool which allows the performing of various procedures for a user entered protein sequences. | ||
|
|
||
| ### Usage |
There was a problem hiding this comment.
Очень классное README, спасибо! Хорошо что написали вклад каждого члена команы:)
5 participants
Add protein_tools.py and update README