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 smertina #7

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

Hw4 smertina #7

wants to merge 22 commits into from

Conversation

sme229
Copy link

@sme229 sme229 commented Sep 30, 2023

HW4 Elena Smertina and Natalia Erofeeva

Copy link

@SidorinAnton SidorinAnton left a comment

Choose a reason for hiding this comment

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

В целом неплохо!
Основные моменты:

  • ридми тоже нужно было сделать в папке HW4
  • импорты всегда должны быть сверху файла
  • Между функцией и докстрокой не надо делать перенос строки. Также не надо его делать в конце докстроки. То есть вместо вот этого:
def func(...):

    """
    text
    
    """

просто вот так:

def func(...):
   """
   text
   """

three_letter_aa += aa_code_dict[aa]
return three_letter_aa

from typing import Optional

Choose a reason for hiding this comment

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

Все импорты должны быть сверху файла ))

elif function == 'find_motifs':
results.append(find_motifs(seq, motif))
if len(results) == 1:
results = results[0]

Choose a reason for hiding this comment

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

return results[0]

Comment on lines +61 to +66
if unique_chars <= single_letter:
seq = 'single_letter_prot_seq'

else:
raise ValueError("Invalid Input")
return seq

Choose a reason for hiding this comment

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

Если у вас тут идет проверка только на валидность (что передали однобуквенную последовательность), то лучше вернуть тут bool (True/False)


1. Посмотрите состав своей команды здесь ([**ССЫЛКА**](https://docs.google.com/spreadsheets/d/1KMBBBu8LqauRpDJb0v1ldPwpvzNn8-KakcHexAcqLsE/edit?usp=sharing)).
2. Тимлид делает форк данного репозитория. **В форке создает ветку `HW4_<surname>`, в ветке создает папку `HW4_<surname>`, в этой папке вы всё делаете.**

Choose a reason for hiding this comment

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

По заданию, собственно, нужно было ридми делать в папке HW4_ ))


"""

# Словарь для хранения частоты аминокислот

Choose a reason for hiding this comment

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

Можно было не писать ))

'T':119, 't':119, 'W':204, 'w':204, 'Y':181, 'y':181, 'V':117, 'v':117}
water_mw = 18
for aa in list_input_seq:
total_mw = sum(aa_weight_dict[a] for a in list_input_seq)

Choose a reason for hiding this comment

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

Зачем это тут?
То есть проходя по аминокислотам мы буквально каждый раз должны считать эту сумму (проходя по аминокислотам) )))
Почему бы не вынести было это до цикла for?

(Говоря на алгоритмическом, это вот уже вообще O(N**2) )

'M':149, 'm':149, 'F':165, 'f':165, 'P':115, 'p':115, 'S':105, 's':105,
'T':119, 't':119, 'W':204, 'w':204, 'Y':181, 'y':181, 'V':117, 'v':117}
water_mw = 18
for aa in list_input_seq:

Choose a reason for hiding this comment

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

Ну и еще один вопрос по циклу ... а зачем он тут вообще нужен, если ниже вот эта aa вообще никак не используется? 0_0
То есть буквально:

        water_mw = 18
        total_mw = sum(aa_weight_dict[a] for a in seq)
        return total_mw - water_mw * (len(seq) - 1)

- str, a 3-letter coded protein sequence without spaces

"""
if check_protein_seq(seq) == 'single_letter_prot_seq':

Choose a reason for hiding this comment

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

Та же история. Тут эта проверка не нужна, раз функций только это и возвращает ))

Comment on lines +103 to +107
aa_code_dict = {'C':'Cys', 'c':'Cys', 'D':'Asp', 'd':'Asp', 'S':'Ser', 's':'Ser', 'Q':'Gln', 'q':'Gln',
'K':'Lys', 'k':'Lys', 'I':'Ile', 'i':'Ile', 'P':'Pro', 'p':'Pro', 'T':'Thr', 't':'Thr',
'F':'Phe', 'f':'Phe', 'N':'Asn', 'n':'Asn', 'G':'Gly', 'g':'Gly', 'H':'His', 'h':'His',
'L':'Leu', 'l':'Leu', 'R':'Arg', 'r':'Arg', 'W':'Trp', 'w':'Trp', 'A':'Ala', 'a':'Ala',
'V':'Val', 'v':'Val', 'E':'Glu', 'e':'Glu', 'Y':'Tyr', 'y':'Tyr', 'M':'Met', 'm':'Met'}

Choose a reason for hiding this comment

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

Это тоже бы вынести как константу в начало

'L':'Leu', 'l':'Leu', 'R':'Arg', 'r':'Arg', 'W':'Trp', 'w':'Trp', 'A':'Ala', 'a':'Ala',
'V':'Val', 'v':'Val', 'E':'Glu', 'e':'Glu', 'Y':'Tyr', 'y':'Tyr', 'M':'Met', 'm':'Met'}

three_letter_aa = ''

Choose a reason for hiding this comment

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

Тогда уж three_letter_seq

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.

3 participants