HW4_Gorbarenko #14
HW4_Gorbarenko #14
Conversation
Add function 'sulphur_containing_aa_counter()' to the 'amino_analyzer.py' tool that counts sulphur-containing amino acids (Cysteine and Methionine) in a protein sequence.
Add 'run_amino_analyzer()' function - the main function to run the amino-analyzer.py tool.
1. Delete repeated elements in code 2. Argument types are specified for 'sulphur_containing_aa_counter()' and 'run_amino_analyzer()' functions
There was a problem hiding this comment.
Хорошая работа!
Плюсы:
- Отличный README. Есть не только примеры, но и возможные причины, почему не работает код. Очень круто.
- Код работает правильно, за исключением функции sulphur_containing_aa_counter.
- К докстрингам претензий нет, подробно написано, что ожидается на вход и что делает каждая функция.
- Здорово, что сделали raise.
Замечания:
- Не всегда после функций есть 2 пропуска строк (чаще одна).
- Лишний цикл for в главной функции.
- Нейминг в целом разумный. НО i - плохой нейминг (особенно в вашем случае, когда i - это аминокислотный остаток). Причем кто-то из вас сделал хорошо и писал не i, а aa, что более разумно.
- Функция sulphur_containing_aa_counter не работает с аминокислотной последовательностью в нижнем регистре.
- Нет единообразия в коде: где-то пишутся одинарные кавычки, где-то двойные; где то аминокислотная последовательность называется sequence, где-то seq. Если над кодом работает не один человек, лучше либо сразу договориться о стиле и о том, как вы будете называть общие переменные. В данном случае это не было столь критично, но если в коде не 1-2 общих переменных, а, скажем, 20 и в разных функциях они называются по-разному, то тут сопоставить переменные по всем функциям будет проблематично.
- Нет единообразия комментариев к коммитам. Некоторые более подробные, некоторые менее. На мой взгляд, наиболее оптимальные комментарии к коммитам выглядят как-то так: Add 'choose_weight' function. Можно в принципе их делать более подробными (Add 'is_aa' function to check if a sequence contains amino acids), но сильно длинные комментарии к коммитам тоже не всегда хорошо.
Баллы:
- README - 2.7 балла
- За функции aa_weight, count_hydroaffinity, count_hydroaffinity, peptide_cutter - 1.5 * 4 = 6 баллов
- За функцию sulphur_containing_aa_counter - 0.5 балла
- Штрафы: нейминг -0.5 балла, лишний for в главной функции -0.3 балла, комментарии к коммитам -0.2 балла, константы не капслоком - 0.5 балла, не всегда 2 пропуска строки после функции -0.2 балла = -1.7 балла
Итого: 7.5 баллов
| Check if a sequence contains only amino acids. | ||
|
|
||
| Args: | ||
| seq (str): The input sequфence to be checked. |
There was a problem hiding this comment.
| seq (str): The input sequфence to be checked. | |
| seq (str): The input sequence to be checked. |
| aa_list = ['V', 'I', 'L', 'E', 'Q', 'D', 'N', 'H', 'W', 'F', 'Y', 'R', 'K', 'S', 'T', 'M', 'A', 'G', 'P', 'C', | ||
| 'v', 'i', 'l', 'e', 'q', 'd', 'n', 'h', 'w', 'f', 'y', 'r', 'k', 's', 't', 'm', 'a', 'g', 'p', 'c'] |
| aa_list = ['V', 'I', 'L', 'E', 'Q', 'D', 'N', 'H', 'W', 'F', 'Y', 'R', 'K', 'S', 'T', 'M', 'A', 'G', 'P', 'C', | ||
| 'v', 'i', 'l', 'e', 'q', 'd', 'n', 'h', 'w', 'f', 'y', 'r', 'k', 's', 't', 'm', 'a', 'g', 'p', 'c'] | ||
| unique_chars = set(seq) | ||
| amino_acids = set(aa_list) |
There was a problem hiding this comment.
Лучше тогда завести сразу set, вы же его сами задали.
| average_weights = [71.0788, 156.1875, 114.1038, 115.0886, 103.1388, 129.1155, 128.1307, 57.0519, 137.1411, 113.1594, | ||
| 113.1594, 128.1741, 131.1926, 147.1766, 97.1167, 87.0782, 101.1051, 186.2132, 163.1760, 99.1326] |
There was a problem hiding this comment.
-
Вот такая запись мне идейно не нравится. Понятно, что вы в функции
aa_weightсобираете словарь на лету. Но читать это очень неудобно. А вдруг вы где-то ошиблись в весе конкретной аминокислоты? Тогда придется отстирывать до нужного числа, это не очень удобно и читаемо. Лучше сразу сделать тут словарь. -
Можно не создать эти списки с весами с разными названиями переменных, можно было просто их назвать
weights_aaи тогда возвращался бы тот список (а лучше бы словарь), который прошел условие if/elif.
There was a problem hiding this comment.
Просто самый простой способ сделать сначала списки, а потом словарь) (потому что списки в нужном порядке можно найти в интернете, а вот словари - нет :D)
Учту на будущее, спасибо!
Второй комментарий не очень поняла, там же по итогу функция возвращает как раз нужный список
There was a problem hiding this comment.
Смотри, я загуглила словарь с весами, в первой же ссылке нашла, нужный: https://stackoverflow.com/questions/27361877/how-i-can-add-the-molecular-weight-of-my-new-list-of-strings-using-a-mw-dictiona . Списки сложнее спустя время отслеживать, что нигде нет ошибки.
Второй комментарий был про то, что у вас в, например, в if сперва создается список average_weights, а строкой ниже вы создаете переменную weights_aa = average_weights. Здесь просто не нужно создавать отдельные переменные average_weights и monoisotopic_weights.
| Returns: | ||
| float: The calculated weight of the amino acid sequence. | ||
| """ | ||
| aa_list = str('A, R, N, D, C, E, Q, G, H, I, L, K, M, F, P, S, T, W, Y, V').split(', ') |
There was a problem hiding this comment.
Зачем такая запись? Сперва вы берете строку, потом опять ее превращаете в строку, а потом делаете из строки список. Лучше сразу сделать список и объявить его константной переменной (+ вынести из под функции).
| aa_list = str('A, R, N, D, C, E, Q, G, H, I, L, K, M, F, P, S, T, W, Y, V').split(', ') | |
| AA_LIST = ['A', 'R', 'N', 'D', 'C', 'E', 'Q', 'G', 'H', 'I', 'L', 'K', 'M', 'F', 'P', 'S', 'T', 'W', 'Y', 'V'] |
| """ | ||
| counter = 0 | ||
| for i in sequence: | ||
| if i == 'C' or i == 'M': |
There was a problem hiding this comment.
Из-за этой логики, у вас тянется ошибка: вы проверяете только по заглавным буквам, а при выбросе ошибки на аминокислотный словарь в вашей главной функции написано, что "Only amino acids are allowed (V, I, L, E, Q, D, N, H, W, F, Y, R, K, S, T, M, A, G, P, C, v, i, l, e, q, d, n, h, w, f, y, r, k, s, t, m, a, g, p, c".
Поэтому я могу захотеть подать последовательность 'mmmmm' и результат будет 0.
| counter += 1 | ||
| answer = str(counter) | ||
| return 'The number of sulphur-containing amino acids in the sequence is equal to ' + answer | ||
|
|
| for i in sequence: | ||
| if not is_aa(sequence): |
There was a problem hiding this comment.
Цикл for здесь не нужен. Функция is_aa и так превращает строку в set и анализирует последовательность.
| for i in sequence: | |
| if not is_aa(sequence): | |
| if not is_aa(sequence): |
| @@ -0,0 +1,196 @@ | |||
| from typing import List | |||
|
|
|||
There was a problem hiding this comment.
Не библиотека не нужна, я пропуск строки добавила. Насколько помню, в PEP8 допускается пропуск 1 строки после импортов, но почти во всем коде, что я смотрела, видела именно 2 пропуска строки. За это баллы не снижала.
| if sequence[i] in ['K', 'R', 'k', 'r'] and sequence[i+1] not in ['P','p']: | ||
| cleavage_sites.append(i+1) |
There was a problem hiding this comment.
| if sequence[i] in ['K', 'R', 'k', 'r'] and sequence[i+1] not in ['P','p']: | |
| cleavage_sites.append(i+1) | |
| if sequence[i] in ['K', 'R', 'k', 'r'] and sequence[i + 1] not in ['P','p']: | |
| cleavage_sites.append(i + 1) |
No description provided.