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 mukhametshina #29

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

Conversation

1799mrd
Copy link

@1799mrd 1799mrd commented Oct 11, 2023

No description provided.

@1799mrd
Copy link
Author

1799mrd commented Oct 31, 2023

comment

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.

Привет!
Извини пожалуйста что так поздно проверил работу!

В каком то смысле это тоже реальная сторона программирования. Можешь загуглить то сколько мемов есть про approve my pull request, ахах
image
image
image
image

Общий комментарий:

  • В целом хорошая работа, почти везде неплохой код и адекватые идеи функций
  • Есть какие-то проблемки с локальным качеством типа неймингов - это все старался отмечать
  • Есть небольшая проблема с ошибим дизайном и логикой. Ты добавила одну отдельную ветку с рандомом и input() от пользователя. Я конечно скажу что этого делать было не нужно, но если ты просто хотела попрактиковаться с таким делом - то тогда это все окей и наоборот здорово! Но к сожалению то как это написано получилось немного сумбурно с высоты птичьего полета. То есть у тебя прям везде копипастом повторяется код, функции делают много не своей работы. Это окей, мы только учимся. Но вот важно отметить. У тебя неплохой кодинг, там буквально что-то довести до идеала. Но к этому надо добавить еще умение видеть общую структуру проекта. Было бы здорово нарисовать условно весь пайплайн на бумажке до того как писать код. Хотя если често я так никогда не делаю. Просто когда готовишь еду или ходишь в душ - шепотом про себя говори вслух историю твоего проекта. Историю в смысле что происходит с данными, то есть идею и логику. И тогда поняв что говоришь одно и то же пару раз, поймешь где можно чуть по другому пересобрать. Потому что когда пишешь код прямо пальцами - в этот момент нет времени смотреть на дизайн.

Если кратко, то у тебя подход такой:

Дело_1
Если условие:
     Дело_2
Если нет:
    Дело_3
Если условие:
     Дело_4, return
Если нет:
     Дело_5
Если условие:
     Дело_6, return
Если нет:
     Дело_7, return

В этом плане это кажется вполне straightforward, но чуть более по-программистки было бы собрать в следующем стиле

Если условие:
     Сделать такую предобработку
Если условие:
     Сделать другую предобработку
Если условие:
     Сделать другую предобработку

Дело_1
Дело_2
return

Надеюсь у меня получилось это как то описать. Если что по коду тоже оставил комментарии.

В любом случае, ты молодец! Очень здорово что ты прям попробовала покопаться и поделать разные штуки. Ридми крутой.

Баллы

  • README 2.5/2.5
  • Операции над белками 6/7.5 (из-за повторений кода и того что я писал выше)
  • За проблемы с неймингами и т п -0.1
  • За фото команды в README 0.1). Добавил за то что есть, но половину потому что кажется чувак на картинке не принимал участие в этой работе:)

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

Итого: 8.5/10

@@ -0,0 +1,108 @@
[![Будто бы полезная ссылка, но просто попытка вставить ссылку_2]<a href="url"><img src="https://upload.wikimedia.org/wikipedia/commons/thumb/6/66/AminoAcidball_rus.svg/1280px-AminoAcidball_rus.svg.png" align="left" height="128" width="198" ></a>](https://ru.wikipedia.org/wiki/%D0%90%D0%BC%D0%B8%D0%BD%D0%BE%D0%BA%D0%B8%D1%81%D0%BB%D0%BE%D1%82%D1%8B)
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 +8 to +16
## Table of contents

* [Project description](#project-description)
* [Main part](#Main-part)
* [Project description](#project-description)
* [Examples](#examples)
* [Contact](#contact)
* [Educational result](#Учебный-результат)
* [See also](#see-also)
Copy link
Member

Choose a reason for hiding this comment

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

Это супер-здорово, спасибо!

## Project description
This project was supposed to be carried out in a team, but, unfortunately, I was unable to do only part of this project - all the code and the mini-program were written by me independently. As this HW, I refreshed my memory in working through GitHub, as well as the basic concepts of the Python language.

![alone](https://sun9-40.userapi.com/impf/c636016/v636016166/239f1/p0AWqN3onLw.jpg?size=550x483&quality=96&sign=19b32adae4a5ac6a436a740160fed9c6&type=album)
Copy link
Member

Choose a reason for hiding this comment

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

😁😁😁

## Main part
Implemented the program `amino_acid_tools.py `. This program necessarily contains the `amino_acid_tools` function, as well as other functions that are described below. The `amino_acid_tools` function accepts an arbitrary number of arguments with a sequence of amino acids or several amino acid sequences (*str*), and it is also possible to introduce the word "random", which generates a random chain of amino acids, in addition, you must enter the name of the procedure to be performed (this is always the last argument, *str*, see usage example). After that, the command performs the specified action on all the transmitted sequences. If one sequence is submitted, a string with the result is returned. If several are submitted, a list of strings is returned.

**Список процедур:**
Copy link
Member

Choose a reason for hiding this comment

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

Внезапно parle france

Comment on lines +43 to +47
```shell
amino_acid_tools('PLfHnfPdD', 'YsGPFEEt', 'ogknHIPTu', 'long_amino_code')
```
Input: 'PLfHnfPdD', 'YsGPFEEt', 'ogknHIPTu', 'long_amino_code'
Output: '['ProLeuPheHisAsnPheProAspAsp', 'TyrSerGlyProPheGluGluThr', 'PylGlyLysAsnHisIleProThrSec']'
Copy link
Member

Choose a reason for hiding this comment

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

Во, да, так и надо. Чтобы можно было просто скопипастить и прогнать. Только можно наверное вставить оформление не shell, а python. И строчка и Input в целом тут лишняя, у тебя же в коде как раз input и виден:)

aminoacid_charge = {'R': 1, 'D': -1, 'E': -1, 'K': 1, 'O': 1}
charge = 0
for aminoacid in amino_sequencqe.upper():
if aminoacid in 'RDEKO':
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
if aminoacid in 'RDEKO':
if aminoacid in aa_charges:

Copy link
Member

Choose a reason for hiding this comment

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

это будет как раз проверка на наличие в ключах

if aminoacid in 'RDEKO':
charge += aminoacid_charge[aminoacid]
if charge > 0:
return 'positiv'
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
return 'positiv'
return 'positive'

))

А вообще было бы здорово прямо заряд сам и вернуть наверное

Comment on lines +184 to +188
for seq in amino_sequence:
unique_chars = set(seq)
amino_acids = set(amino_acid)
if unique_chars <= amino_acids:
aminoac_seqs.append(seq)
Copy link
Member

Choose a reason for hiding this comment

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

то есть если пользователь добавил что-то неправильное, то он об этом даже и не узнает?

amino_to_rna (str) or (list): possible RNA sequence
amino_seq_charge (str) or (list): "positive", "negative" or "neutral"
"""
*seqs, function = args
Copy link
Member

Choose a reason for hiding this comment

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

Это было актуально в домашке по ДНК/РНК чтобы вы потреннировались с аргументами. Вообще это не очень хорошо, так как по факту сиквенсы и название функции - это разные сущности. А мы их мешаем в одну дырку. Это не хорошо. Есть есть разные штуки - то и принимать их стоит в разные аргументы.

def function(seqs, func):

amino_seq_charge (str) or (list): "positive", "negative" or "neutral"
"""
*seqs, function = args
d_of_functions = {'long_amino_code' : long_amino_code,
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
d_of_functions = {'long_amino_code' : long_amino_code,
functions = {'long_amino_code' : long_amino_code,

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.

2 participants