Skip to content

Hw4 Uzun #26

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

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

Hw4 Uzun #26

wants to merge 39 commits into from

Conversation

uzunmasha
Copy link

No description provided.

uzunmasha and others added 30 commits September 26, 2023 19:14
Add a description of protein_length and  essential_amino_acids functions. 
Add Bibliography
Replace double quotes with single quotes
Correct essential_amino_acids function description
Change function input arguments to general form
Copy link
Member

@nvaulin nvaulin left a comment

Choose a reason for hiding this comment

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

Привет!

❗️ Напоминание напомнить всем членам команды посмотреть фидбек.

Ниже можете найти подробные комментарии по коду. Тут некоторые общие моменты:

  1. Очень классный README. Хорошая структура и примеры (только один пример работает не так как у вас заявлено))
  2. Не очень хорошая структура коммитов) Например вот тут у вас и сообщение не очень, и за один коммит добавлена куча кода Add code
  3. По коду. Были проблемы с неймингами, обратите прям внимание. У вас были места где это прям важно. Тем не менее почти везде всё хорошо. Хорошая логика в целом (если не считать небольшой проблемки с функцией по подсчету длины белка).
  4. По форматам выводов. У вас возвращаются списки. В целом это ок (разве что мб когда один белок на входе, то стоит давать просто ответ, а не ответ в списке). Но можно было бы сделать еще и так. Возвращать словарь (ключ - белок в запросе, значение - результат). Но это просто как идея)

Баллы

  • README 2.5/2.5 + 0.2
  • Операция length 1.3/1.5 (работает немного не так как надо бы)
  • Остальные операции 1.5*4 = 6/6

За структуру коммитов -0.5
За качество кода -0.5

Итого: 9

@@ -0,0 +1,181 @@
def protein_mass(seq: str):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def protein_mass(seq: str):
def calculate_protein_mass(seq: str) -> float:

Amino acids in the string should be indicated as one-letter symbols.

"""
aa_seq = list(seq.upper())
Copy link
Member

Choose a reason for hiding this comment

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

Не обязательно было переводить это дело в список)

def protein_mass(seq: str):
"""

Calculate the mass (Da) of a protein based on its amino acids sequence.
Copy link
Member

Choose a reason for hiding this comment

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

Правильно что указали размерность!


"""
aa_seq = list(seq.upper())
mass_dictionary = dict({'A': 89, 'R': 174, 'N': 132, 'D': 133, 'C': 121, 'Q': 146, 'E': 147, 'Z': 147,
Copy link
Member

Choose a reason for hiding this comment

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

  1. Не нужно тут писать dict, у нас уже {... : ...} это словарь
  2. Такие вещи лучше выносить в начало кода. Это называют константами и нейминг у них идет целиком капсом. Мы это подробно обсуждали на консультации 03.10.23, советую посмотреть

Copy link
Member

Choose a reason for hiding this comment

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

И не очень принято добавлять тип переменной в ее название. Хотя бы не целиком, а dict. Ну а вообще питонисто было бы как-то так:

Suggested change
mass_dictionary = dict({'A': 89, 'R': 174, 'N': 132, 'D': 133, 'C': 121, 'Q': 146, 'E': 147, 'Z': 147,
AA_MASSES: dict = dict({'A': 89, 'R': 174, 'N': 132, 'D': 133, 'C': 121, 'Q': 146, 'E': 147, 'Z': 147,

Comment on lines +13 to +17
mass = 0
for amino_acid in aa_seq:
mass += mass_dictionary[amino_acid]

return mass
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mass = 0
for amino_acid in aa_seq:
mass += mass_dictionary[amino_acid]
return mass
mass = 0
for amino_acid in aa_seq:
mass += mass_dictionary[amino_acid]
return mass

Comment on lines +28 to +30
This function calculates the mass (Da) of a protein based on its amino acid sequence. As input, it takes a string of amino acids and returns the molecular weight in Da.
Usage example:
```python
Copy link
Member

Choose a reason for hiding this comment

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

В Markdown как и в latex чтобы сделать новый абзац надо оставить пустую строку

Suggested change
This function calculates the mass (Da) of a protein based on its amino acid sequence. As input, it takes a string of amino acids and returns the molecular weight in Da.
Usage example:
```python
This function calculates the mass (Da) of a protein based on its amino acid sequence. As input, it takes a string of amino acids and returns the molecular weight in Da.
Usage example:

aa_tools('KKNNfF', 'KKFFRRVV', 'KK', 'essential_amino_acids') #[['K', 'K', 'f', 'F'], ['K', 'K', 'F', 'F', 'V', 'V'], ['K', 'K']]
```

## Troubleshooting
Copy link
Member

Choose a reason for hiding this comment

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

Круто что есть такая секция. Это можно еще проработать в коде в виде осмысленных ошибок

* In functions `'amino_acid_substring'` and `'amino_acid_count'` output [-1] means that there is no such element in the sequence.
* In functions `'amino_acid_substring'` and `'amino_acid_count'` the error message "name '..' is not defined" means that the given argument is not quoted in the input string.

## Bibliography
Copy link
Member

Choose a reason for hiding this comment

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

🔥

* Maria Uzun - contributed to `'amino_acid_substring'`, `'amino_acid_count'`, and `'aa_tools'` functions.
* Maria Babaeva - contributed to `'protein_mass'` and `'amino_acid_profile'` functions.
* Kristina Zhur - contributed to `'protein_length'` and `'essential_amino_acids'` functions.
* Julia the Cat - team's emotional support.
Copy link
Member

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 +101
for sequences in seq:
lengths.append(len(sequences))
Copy link
Member

Choose a reason for hiding this comment

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

У вас же цикл по сиквенсам уже есть в главной функции
Тогда получается не совсем правильная работа
image

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.

5 participants