Skip to content

Conversation

@Alisa411
Copy link
Owner

@Alisa411 Alisa411 commented Oct 8, 2023

No description provided.

Alisa411 and others added 30 commits October 8, 2023 15:26
Comment on lines +2 to +3
import protein_dict as prd
from random import choice

Choose a reason for hiding this comment

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

Вот тут все супер, protein_dict лежит в той же директории. что и программа, все импортируется, красота

Comment on lines +84 to +86
seq = seq.upper()
sequence = "".join(prd.AA_ONE_TO_THREE_LETTER[aa] for aa in seq)
return sequence[:-1]

Choose a reason for hiding this comment

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

Тоже нет улучшения, но при этом в словаре "-" в конце каждой а\к отсутствует, получается, от последней а\к будет откусываться буква?...

@@ -0,0 +1,95 @@
# Import dna_rna_dict.py containing dictionaries for working with dna and rna sequences
import dna_rna_dict as drd

Choose a reason for hiding this comment

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

Вот тут все тоже сработает, тк словарь лежит в той же директории. что и программа, все импортируется

Comment on lines +73 to +75
action = args[-1].lower()
sequences = args[:-1]
results = []

Choose a reason for hiding this comment

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

ой, а где улучшение кода (не критично)

"""
total_offset = 0
for char in quality_string:
offset = ord(char) - 33 # Assuming 33 as the default encoding offset

Choose a reason for hiding this comment

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

отлично!

Comment on lines +75 to +79
if not isinstance(gc_bounds, tuple):
gc_bounds = (0, gc_bounds)

if not isinstance(length_bounds, tuple):
length_bounds = (0, length_bounds)

Choose a reason for hiding this comment

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

Отличная проверка!

Comment on lines +83 to +85
if not all(letter in drd.DNA_LETTERS for letter in sequence):
print(f"Skipping non-fastq sequence: {seq_name}")
continue

Choose a reason for hiding this comment

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

Хорошая проверка! В целом, когда потом будет туда подгружать fastq-файлы, она не столь обязательна (но можно оставить), вообще хорошая идея проверять все и вся

Comment on lines +102 to +104
print(filtered_seqs)

return filtered_seqs

Choose a reason for hiding this comment

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

Ой, а зачем нам и выводить, и возвращать? В задании же только вернуть словарь надо

Comment on lines +1 to +3
from data_processing_scripts.dna_rna_tools import main_dna_rna_tools
from data_processing_scripts.das_protein_tools import main_protein_tools
from data_processing_scripts.fastq_tools import main_fastq_tools

Choose a reason for hiding this comment

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

импорты корректные, даже после отделены двумя пустыми строками. все по PEP8, здорово

А вот дальше когда вызываются функции, то они обращаются к словарям и пытаются их импортировать. а в этой папке словарей нет, и все падает

Copy link
Owner Author

Choose a reason for hiding this comment

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

То есть сюда же нужно прописать импорт словарей? Или же словари поднять на директорию выше, где лежит скрипт вызова функций?

Comment on lines +7 to +27
result = main_dna_rna_tools("ATcg", "reverse")
print(result)

# Call the main_protein_tools function with the necessary arguments.
result = main_protein_tools("ACDE", "protein_mass")
print(result)

# Call the main_fastq_tools function with the necessary arguments.
EXAMPLE_FASTQ = {
'@SRX079804:1:SRR292678:1:1101:21885:21885': (
'ACAGCAACATAAACATGATGGGATGGCGTAAGCCCCCGAGATATCAGTTTACCCAGGATAAGAGATTAAATTATGAGCAACATTATTAA',
'FGGGFGGGFGGGFGDFGCEBB@CCDFDDFFFFBFFGFGEFDFFFF;D@DD>C@DDGGGDFGDGG?GFGFEGFGGEF@FDGGGFGFBGGD'),
}
filtered_sequences = main_fastq_tools(
seqs=EXAMPLE_FASTQ,
gc_bounds=(0, 80), # GC content от 40% до 60%
length_bounds=(10, 100), # Длина последовательности от 10 до 100
quality_threshold=0 # Порог качества
)

print(filtered_sequences)

Choose a reason for hiding this comment

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

Но даже если бы не падало - пользователь при запуске скрипта сразу получит... Выводы тестовых запусков? Зачем?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Это было скорее для примера работы, чтобы при проверке посмотреть. Как это лучше оформить?

@pavlovanadia
Copy link

Комментарии:

  • README в ветке отсутствует
  • Структура репозитория, откровенно говоря, так себе. То есть в целом если скрипт со словарями лежит в папке - ээто допустимо, но почему один из словарей оказался на уровень выше? Выглядит непродуманно. к тому же один и тот же словарь повторяется на обоих уровнях...
  • Основной скрипт, видимо, не был протестирован - при первой же попытке запуска ошибка выдает неправильную структуру импортов словарей (подробнее далее)
  • Из основного скрипта верно импортируются три главные функции вспомогательных скриптов (и это здорово!). по идее главные функции Никита просил написать в основном скрипте - сам по себе этот факт я ошибкой не считаю, но именно нынешний вариант решения, а не как советовал Никита, приводит в дальнейшем в ошибке
  • Три главные функции используют словари, лежащие в той же папке, что и сами функции. НО вызываются они в главном скрипте. И - соответственно - при импорте словарей происходит ошибка, потому что словари не были импортированы в скрипт предварительно, а код их теперь, при вызове главных функций из основного скрипта, будет ожидать видеть в той же папке, что и основной скрипт. Поэтому с ошибкой падает все (ну кроме импорта словаря для нуклеиновых кислот, который почему-то оказался в одной директории с основным скриптом). Подробнее оставила комменты у импортов во всех скриптах, надеюсь, поможет разобраться в этом
  • В основном скрипте написан код для вызова тестовых запусков, непонятно, зачем
  • В целом функции работают не так, как задано в задании. Предполагалось, что каждая функция будет делать фильтрацию - конечно, баллы за это не снижаю, т.к. все получается, но лучшее понимание взаимодействия функций и структуры кода получилось бы, если следовать инструкциям Никиты

Итог:

  • За каждую из 3-x фильтраций fastq - 1 балл.
  • За главную функцию fastq-фильтратора - 0.3 балла, не работает из главного скрипта
  • За README - 0 баллов
  • За улучшения кода ДНК/РНК и белковых тулов - 0.8 балла
  • За структуру репозитория и качество кода - 2 балла

Бонусы и штрафы:

  • +0.1 за хорошую проверку в главной функции на режим границ для фильтрафии по длине и ГЦ-составу
  • +0.1 за проверку на послед-ть в fastq-файлах
  • +0.1 за хороший расчет качества рида

Итог: 6.4

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