-
Notifications
You must be signed in to change notification settings - Fork 0
File input for BSAT #2
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
base: main
Are you sure you want to change the base?
Conversation
nvaulin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Привет!
Хорошая работа
- Хороший README. В целом в таких больших ридми можно не прям все сопровождать примерами, а какую-то часть.
- Хорошие названия коммитов.
- Чтение и запись FASTQ-файлов в фильтраторе кажется работает не совсем так как нужно. Как минимум на выходе окаызываются все сиквенсы целиком). Хотя кажется почти все работает правильно, где-то надо что-то подправить.
- Две другие функции работают тоже хорошо, это вообще супер, ты молодец.
- Очень очень много комментариев по коду. Посмотри все пожалуйста. У тебя почти все окей, но вот такие мелкие моментики не дают довести это все дело до идеала. Где то где комменты повторяются я уже не стал их отмечать по много раз.
Баллы
- Добработка FASTQ-модуля: 1/2 балла
- convert_multiline_fasta_to_oneline: 4/4 балла
- select_genes_from_gbk_to_fasta: 4/4 балла
-0.5 за общее качество кода, вещи по типу проверок, нелогичных неймингов и т д. Старался везде помечать где как лучше сделать.
В любом случае - очень понравилась работа:)
Итого: 8.5 баллов
| from modules_for_BSAT import fastq_analysis as fq | ||
| from modules_for_BSAT import dna_rna_analysis as na | ||
| from modules_for_BSAT import protein_analysis as pa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Выглядит красиво! Только наверное na не очень хорошее название, сразу кажется что это что-то из статистики
| if operation in OPERATION_DICT.keys(): | ||
| analysis.append(OPERATION_DICT[operation](seq)) | ||
| else: | ||
| raise ValueError(f'Wrong operation!') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| raise ValueError(f'Wrong operation!') | |
| raise ValueError(f'Unsupported operation {operation}!') |
Чуть добавим заботы о пользователе:)
| def analyse_fastq(input_path: str, | ||
| gc_bounds: Union[int, float, Tuple [int], Tuple [float]] = (0, 100), | ||
| length_bounds: Union[int, Tuple [int]] = (0, 2**32), | ||
| quality_threshold: float = 0.0, filtered_file_name: Union[None, str] = None) -> Dict[str,str]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Аннотация просто мощь. Только название - все таки мы их не анаизируем, а фильтруем - filter_fastq
| :raises ValueError: if sequence not RNA or DNA, also if the argument values are outside the allowed ones | ||
| """ | ||
| seqs, path_to_file = fq.read_fastq(input_path) | ||
| file_name = path_to_file.split("/")[-1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это ок, но мало ли у человека винда. Для этого есть вообще крутая штука:
| file_name = path_to_file.split("/")[-1] | |
| file_name = os.basename(path_to_file) |
| seqs, path_to_file = fq.read_fastq(input_path) | ||
| file_name = path_to_file.split("/")[-1] | ||
| if type(gc_bounds) == float or type(gc_bounds) == int: | ||
| gc_bounds = (0,gc_bounds) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| gc_bounds = (0,gc_bounds) | |
| gc_bounds = (0, gc_bounds) |
| for key, item in gene_and_seq.items(): | ||
| new_seq_fasta.write(key + '\n') | ||
| new_seq_fasta.write(item + '\n') | ||
| return 'All sequences processed!' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
В IT еще обычно это дело делают через print, а возвращают код 1 или 0
Хотя тут не обязательно в целом что-то возвращать:)
| gene_and_seq = dict() | ||
| with open (input_fasta) as seq_fasta: | ||
| for line in seq_fasta: | ||
| if gene == 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не совсем понятно что значит gene 1 или 0. То есть я понимаю что для тебя это флаг, и тут это ок, но можно было бы его как то более информативно задать. Типо is_new_gene, там True / False, что-то в таком духе.
Опять же, так ок, просто пища для размышлений
| if input_gbk.find('.gbk') == 0: | ||
| raise ValueError(f'Wrong file format in input!') | ||
| if os.path.exists(os.path.join('.', 'Analyzed_data')) == False: | ||
| os.mkdir(os.path.join('.', 'Analyzed_data')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут аналогичные комменты
| for el in genes_for_search: | ||
| genes_for_search_in_gbk += [gn for gn in genes_gbk if el in gn] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут немного конечно праздник двухбуквенных переменных)))
Но в целом окей:)
|
|
||
| ## Contact | ||
|
|
||
| *This is the repo for the 5th homework of the BI Python 2023 course* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не только:)
In this branch we configured the scripts to work with files as input.
P.S. The commit was made to the main branch so as not to accept the previous request and not lose comments on it.