Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HW4 Sivtsev #21

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Conversation

OtterLawyer
Copy link

Homework №4.

Comment on lines +1 to +6
aminoacid_alphabet_1to3 = {'A': 'Ala', 'R': 'Arg', 'N': 'Asn', 'D': 'Asp', 'C': 'Cys',
'Q': 'Gln', 'E': 'Glu', 'G': 'Gly', 'H': 'His', 'I': 'Ile',
'L': 'Leu', 'K': 'Lys', 'M': 'Met', 'F': 'Phe', 'P': 'Pro',
'S': 'Ser', 'T': 'Thr', 'W': 'Trp', 'Y': 'Tyr', 'V': 'Val'}

molecular_mass = {'A': 89.094, 'R': 174.203, 'N': 132.119, 'D': 133.104, 'C': 121.154,

Choose a reason for hiding this comment

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

Поскольку эти переменные являются константами, то их стоит назвать заглавными буквами

bool if sequence is correct
ValueError('Please check proteins sequences') if there were wrong symbols
"""
aas = {'A', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'K', 'L', 'M', 'N', 'P', 'Q', 'R', 'S', 'T', 'V', 'W', 'Y'}

Choose a reason for hiding this comment

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

Стоит вынести как константу

Comment on lines +22 to +23
prot = prot.upper()
uniq_aas = set(prot)

Choose a reason for hiding this comment

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

Можжно в одну строку, чтобы память не забивать prot-ом

Comment on lines +24 to +27
aa_test = (uniq_aas <= aas)
if aa_test == 0:
raise ValueError('Please check proteins sequences')
return True

Choose a reason for hiding this comment

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

Лучше использовать функционал множеств, и результат операции не записывать в отдельную переменную

Suggested change
aa_test = (uniq_aas <= aas)
if aa_test == 0:
raise ValueError('Please check proteins sequences')
return True
aa_test = (uniq_aas <= aas)
if uniq_aas.issubset(aas):
return True
else:
raise ValueError('Please check proteins sequences')

Comment on lines +39 to +45
if len(prot) > 0:
for i in prot:
if i in aminoacid_alphabet_1to3:
output += aminoacid_alphabet_1to3[i]
else:
raise ValueError('Input format: aminoacids in uppercase 1-letter symbols')
return output

Choose a reason for hiding this comment

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

Очень много уровней вложенности
У вас же есть проверка на то, является ли послед-ть белком, зачем внутри писать другую проверку, тем более итерируясь по каждой а\к отдельно? Довольно неоптимально
Тем более в главной фукнции вы эту проверку is_prot используете, у вас нет шанса при вызове этой функции выйти в ошибку, а строк вложенных написано много

else:
for i in prot_seq:
output += prot.count(i) * molecular_mass[i]
output -= 18.0153*(len(prot)-1)

Choose a reason for hiding this comment

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

PEP8 грустит

Suggested change
output -= 18.0153*(len(prot)-1)
output -= 18.0153 * (len(prot) - 1)

Return:
-int - the result of the count
"""
return len(prot)*3

Choose a reason for hiding this comment

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

PEP8 грустит

Suggested change
return len(prot)*3
return len(prot) * 3

Return: aa_content (dict) - dict of aminoacids and their quantity in protein
"""

aas = 'ACDEFGHIKLMNPQRSTVWY'

Choose a reason for hiding this comment

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

по идее должна быть как константа, более того, лучше уж сетом

"""

aas = 'ACDEFGHIKLMNPQRSTVWY'
prot = prot.upper()

Choose a reason for hiding this comment

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

надо бы это делать в главной функции, а то в маленьких функциях то есть, то нет, и в целом это лучше в основной сделать до вызова нужной операции

Comment on lines +100 to +102
for i in range(len(prot)):
n = aas.index(prot[i])
aa_counter[n] += 1

Choose a reason for hiding this comment

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

Лучше через словарь

Return: e (int) - result of counts: extinction coefficient at 280 nm

"""
aa_cont_dict = count_aa_content(prot)

Choose a reason for hiding this comment

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

Здорово, что переиспользовали имеющуюся функцию!)

Comment on lines +122 to +124
W_number = aa_cont_dict.get('W')
Y_number = aa_cont_dict.get('Y')
C_number = aa_cont_dict.get('C')

Choose a reason for hiding this comment

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

С заглавной буквы переменные не надо называть

Comment on lines +126 to +129
if C_number == 0:
e = 5500 * W_number + 1490 * Y_number
else:
e = 5500 * W_number + 1490 * Y_number + 125*(C_number//2)

Choose a reason for hiding this comment

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

Но ведь это буквально одно и то же....

return e


def protein_tools (function : str, *prots : str) -> (int, list, str):

Choose a reason for hiding this comment

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

Эх, а можно было бы операцию в конце указать, так было бы интереснее функцию писать....(

@pavlovanadia
Copy link

Неплохая работа!
Советую обратить внимание на константы, названия функций. Также помните, что есть код повторяется, то скорее всего, можно этого избежать, сделав его лучше.

  • Из текста README не очень понятно, как запускать программу, какое имя основной функции, какие аргументы, в каком количестве и порядке она принимает. Типы данных на вход и выход не указаны. Не очень user-friеndly.
  • Используйте функционал сетов, они хорошие!
  • Здорово, что у каждой функции есть типизация и докстринг!
  • Периодически в коде встречаются ненужные операции, или же ответвления, куда невозможно попасть
  • Структура репозитория нарушена: README находится не в папке проекта, а в папке домашнего задания, по идее, все файлы должны быть в папке с кодом
  • Классное фото команды!)

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.

4 participants