Skip to content

Hw18#4

Open
zmitserbio wants to merge 36 commits into
mainfrom
HW18
Open

Hw18#4
zmitserbio wants to merge 36 commits into
mainfrom
HW18

Conversation

@zmitserbio
Copy link
Copy Markdown
Owner

No description provided.

Add create_results_dir_if_doesnt_exist function
Copy link
Copy Markdown

@ANugmanova ANugmanova left a comment

Choose a reason for hiding this comment

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

Поскольку заданий с потоками и с тестами нет, то оцениваю только репозиторий.

Из хорошего: коротко и по делу:) Довольно понятный нейминг, по коммитам можно понять какие вносились изменения, да и в целом, код довольно аккуратный, тут молодец!

Дальше несколько проблем по стилю, которые повлияли на оценку:

  1. Код иногда идет сплошным полотном. Там, где длинные функции и не хочется их разбивать (хотя лучше почаще разбивать!), нужно делить код на логические блоки и разделять пустыми строками. К примеру, ошибки отдельно, вычисления отдельно, какая-то подготовка результата тоже отдельно. Так и тебе, и кому-то со стороны будет комфортнее читать код. При разработке будет понятно, что нужно сфокусироваться вот на этом кусочке, а не на всей функции. 
И еще важно, там где это возможно, обязательно уходить от вложенностей. Это не только упрощает чтение, но и иногда спасает от ошибок. Поставил слишком мало отступов и операция уже выполняется во внешнем цикле, а не во внутреннем, так что чем меньше циклов в цикле, тем безопаснее:)
  2. Пару раз импорты встречались в очень странных местах, их всегда нужно располагать вначале скрипта.
  3. Не хватает описания. Цель этого задания была в том, чтобы сделать репозиторий, который мог бы посмотреть человек со стороны. Пока что он выглядит не очень дружелюбно для людей, которые не в контексте)
  4. Местами отсутствуют достриги
  5. Не хватает примеров для OpenFasta.

Итого, 9/15 за репозиторий

Comment thread README.md
Homework for Python course in the Bioinformatic Institute
# Homework repository for 2023-2024 Informatics Bioinstitute professional retraining program

Here, several homeworks conserning processing bioinformatical data are gathered. Please, see Showcases.ipynb for some examples.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ну тут все-таки немножно лучше добавить о том что реализовано:) Обработка каких данных, что за обработка вообще. То есть после README должно быть какое-то представление о том полезен человеку этот репозиторий или нет

Comment thread beginner_bioinf_tools.py
return:
- no return
"""
import os
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

импорты тут располагать точно не стоит, все должно быть вначале, ну и у тебя уже есть эта библиотека:)

Comment thread beginner_bioinf_tools.py
- no return
"""
import os
if '.fastq' in os.path.basename(path):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Comment thread beginner_bioinf_tools.py


def create_results_dir_if_doesnt_exist() -> None:
import os
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

это опять-таки отсюда надо убрать

Comment thread beginner_bioinf_tools.py
log = 'stdout:\n\n' + stdout.getvalue() + '\nstderr:\n\n' + stderr.getvalue()
bot_send_message(chat_id, message, function_name, log)

def bot_send_message(chat_id: int, message: str, function_name: str, log: str = None):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Comment thread bio_files_processor.py
self.current_line = None
self.finished = False

def read_record(self):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

эти методы лучше определить после дандер методов

Comment thread requirements.txt
@@ -0,0 +1,3 @@
Bio==1.6.2
biopython==1.83
numpy==1.24.3
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

python-dotenv тоже лучше добавить

Comment thread Showcases.ipynb
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Поскольку в твоем репозитории ноутбук заменяет README, то лучше было бы добавить некоторые пояснения, что вот тут вот мы тестируем базовые операции для днк, рнк и белков, вот тут проверяем, что срабатывает ошибка и тд. Хорошо бы, чтобы человеку, который не очень понимает о чем код при быстром просмотре стало все понятно.

Comment thread README.md
Homework for Python course in the Bioinformatic Institute
# Homework repository for 2023-2024 Informatics Bioinstitute professional retraining program

Here, several homeworks conserning processing bioinformatical data are gathered. Please, see Showcases.ipynb for some examples.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ну тут все-таки стоит побольше рассказать про то что в репозитории, с какими данными работаешь, какой есть функционал. После просмотра README у человека должно быть понимание интересно ему дальше разбираться с репозиторием или нет:)

Comment thread beginner_bioinf_tools.py
class AminoAcidSequence(BiologicalSequence):
alphabet = 'ARNDCQEGHILKMFPSTWYVarndcqeghilkmfpstwyv'

def is_correct_alphabet(self) -> bool:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

это нужно сдвинуть ближе к концу класса. Лучше всего, сначала дандер методы, потом публичные, потом приватные

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