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 petrikov #17

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

Conversation

KirPetrikov
Copy link

No description provided.

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.

В целом неплохо!
Мне кажется, немного перемудрили с функциями для подсчета массы ... но а вообще молодцы ))

* Muradova Gulgaz
* Yury Popov

![Developers](https://github.com/KirPetrikov/HW4_Functions2/blob/HW4_Petrikov/HW4_Petrikov/images/pic.jpg "We are here")

Choose a reason for hiding this comment

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

Не оч понял, а кто 4-ый девелопер )))
Причем судя по его гитхабу, он вроде как пишет на Го )))

Comment on lines +209 to +211
"""
Perform some simple operations on amino acids sequences.
"""

Choose a reason for hiding this comment

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

Раз это главная функция (в какой-то степени точка входа), тут бы побольше описания дать...

'light': find_lightest_proteins}


def process_seqs(option: str, seqs: list):

Choose a reason for hiding this comment

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

def process_seqs(option: str, seqs: List[str]):

Comment on lines +212 to +214
if isinstance(seqs, str):
seq_tmp = seqs
seqs = [seq_tmp]

Choose a reason for hiding this comment

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

Ну вот по идее такого никогда не должно быть, т.к. вы явно сказали, что seqs -- лист )))
Непонятно, зачем проверять её на строчку...

return f'{proteins} - {min_weight}'


def check_sequences(seqs: list):

Choose a reason for hiding this comment

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

def check_sequences(seqs: List[str]):

Comment on lines +130 to +138
def find_heaviest_proteins(sequence: list):
"""
Return the sequence of the heaviest protein from list
"""
protein_mass = {}
list_of_protein = sequence
for i in list_of_protein:
protein_mass[i] = calc_protein_mass(i)
return count_uniq_max_mass(protein_mass)

Choose a reason for hiding this comment

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

Все те же самые комменты, что и для функции find_lightest_proteins ))

Calculate protein molecular weight using the average
molecular weight of amino acid - 110 Da
"""
return len(seq) * 110

Choose a reason for hiding this comment

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

У вас же есть функция sequence_length... почему не использовали её?

return sequence_length(seq) * 110

sequence to 3-letter symbols separated by
hyphens.
"""
new_name = ''

Choose a reason for hiding this comment

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

кажется, не оч удачное название ))
мб хотя бы three_letter_seq

return round(ph_iso_point, 1)


def transform_to_three_letters(seq: str) -> str:

Choose a reason for hiding this comment

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

Имхо, лучше было бы разделитель как раз задать параметром по умолчанию

Comment on lines +89 to +91
charged_amino_ac_numbers = []
for amino_ac in ("C", "D", "E", "Y", "H", "K", "R"):
charged_amino_ac_numbers.append(seq.count(amino_ac))

Choose a reason for hiding this comment

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

Тут не оч хорошо использовать список. Лучше бы сделать тут вот так:

    charged_amino_ac_numbers = {
        "C": 0, "D": 0, "E": 0, "Y": 0, "H": 0, "K": 0, "R": 0
    }
    for amino_ac in seq:
        if amino_ac in charged_amino_ac_numbers:
            charged_amino_ac_numbers[amino_ac] += 1

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